From 191540f2017d19096252a4fda1d5e175cb1e4787 Mon Sep 17 00:00:00 2001
From: pd-redis <petar.dzhambazov@redis.com>
Date: Mon, 24 Nov 2025 20:30:54 +0200
Subject: [PATCH 1/3] refactor: improve RiPopover API with trigger and
 className props for better extensibility

Refactor RiPopover to add optional trigger and className props while maintaining backwards compatibility. React elements in trigger render directly without span wrapper for styled-components support.
---
 .../src/components/base/popover/RiPopover.tsx | 84 +++++++++++++------
 .../ui/src/components/base/popover/types.ts   |  9 +-
 2 files changed, 67 insertions(+), 26 deletions(-)

diff --git a/redisinsight/ui/src/components/base/popover/RiPopover.tsx b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
index 38507e6c0d..31ba60b752 100644
--- a/redisinsight/ui/src/components/base/popover/RiPopover.tsx
+++ b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
@@ -11,34 +11,68 @@ export const RiPopover = ({
   children,
   ownFocus,
   button,
+  trigger,
   anchorPosition,
   panelPaddingSize,
   anchorClassName,
   panelClassName,
+  className,
   maxWidth = '100%',
   ...props
-}: RiPopoverProps) => (
-  <Popover
-    {...props}
-    open={isOpen}
-    onClickOutside={closePopover}
-    onKeyDown={(event) => {
-      // Close on escape press
-      if (event.key === keys.ESCAPE) {
-        closePopover?.(event as any)
-      }
-    }}
-    content={children}
-    // Props passed to the children wrapper:
-    className={panelClassName}
-    maxWidth={maxWidth}
-    style={{
-      padding: panelPaddingSize && panelPaddingSizeMap[panelPaddingSize],
-    }}
-    autoFocus={ownFocus}
-    placement={anchorPosition && anchorPositionMap[anchorPosition]?.placement}
-    align={anchorPosition && anchorPositionMap[anchorPosition]?.align}
-  >
-    <span className={anchorClassName}>{button}</span>
-  </Popover>
-)
+}: RiPopoverProps) => {
+  // Warn if both button and trigger are provided
+  if (button !== undefined && trigger !== undefined) {
+    console.warn(
+      "RiPopover: Both 'button' and 'trigger' props are provided. Using 'trigger'. Please migrate to 'trigger' prop.",
+    )
+  }
+
+  // Warn if both panelClassName and className are provided
+  if (panelClassName !== undefined && className !== undefined) {
+    console.warn(
+      "RiPopover: Both 'panelClassName' and 'className' props are provided. Using 'className'. Please migrate to 'className' prop.",
+    )
+  }
+
+  // Determine which trigger to use
+  const activeTrigger = trigger ?? button
+
+  // Determine which className to use
+  const activeClassName = className ?? panelClassName
+
+  // Render trigger element
+  // For new API (trigger): React elements render directly, scalars wrap in span
+  // For old API (button): Always wrap in span (backwards compatibility)
+  const triggerElement =
+    trigger !== undefined && React.isValidElement(activeTrigger) ? (
+      activeTrigger
+    ) : (
+      <span className={anchorClassName}>{activeTrigger}</span>
+    )
+
+  return (
+    <Popover
+      {...props}
+      open={isOpen}
+      onClickOutside={closePopover}
+      onKeyDown={(event) => {
+        // Close on escape press
+        if (event.key === keys.ESCAPE) {
+          closePopover?.(event as any)
+        }
+      }}
+      content={children}
+      // Props passed to the children wrapper:
+      className={activeClassName}
+      maxWidth={maxWidth}
+      style={{
+        padding: panelPaddingSize && panelPaddingSizeMap[panelPaddingSize],
+      }}
+      autoFocus={ownFocus}
+      placement={anchorPosition && anchorPositionMap[anchorPosition]?.placement}
+      align={anchorPosition && anchorPositionMap[anchorPosition]?.align}
+    >
+      {triggerElement}
+    </Popover>
+  )
+}
diff --git a/redisinsight/ui/src/components/base/popover/types.ts b/redisinsight/ui/src/components/base/popover/types.ts
index f25792706f..da361ef10c 100644
--- a/redisinsight/ui/src/components/base/popover/types.ts
+++ b/redisinsight/ui/src/components/base/popover/types.ts
@@ -1,6 +1,7 @@
 import { type PopoverProps } from '@redis-ui/components'
 
 import { anchorPositionMap, panelPaddingSizeMap } from './config'
+import { ReactNode } from 'react'
 
 type AnchorPosition = keyof typeof anchorPositionMap
 
@@ -19,10 +20,16 @@ export type RiPopoverProps = Omit<
   isOpen?: PopoverProps['open']
   closePopover?: PopoverProps['onClickOutside']
   ownFocus?: PopoverProps['autoFocus']
-  button: PopoverProps['content']
+  /* @deprecated old prop for popover trigger element, use trigger */
+  button?: PopoverProps['content']
+  /* preferred prop for popover trigger element (optional) */
+  trigger?: ReactNode
   anchorPosition?: AnchorPosition
   panelPaddingSize?: PanelPaddingSize
   anchorClassName?: string
+  /* @deprecated - use @see{className} - this is popover content wrapper class name */
   panelClassName?: string
+  // new preferred prop for popover content wrapper class name (optional)
+  className?: string
   'data-testid'?: string
 }

From e6e8ad76e140978c4c0161aff1ed2b1b0c436f98 Mon Sep 17 00:00:00 2001
From: pd-redis <petar.dzhambazov@redis.com>
Date: Mon, 24 Nov 2025 21:07:16 +0200
Subject: [PATCH 2/3] refactor: improve RiPopover API with trigger and
 className props for better extensibility

Refactor RiPopover to replace `button` with `trigger` and className props while maintaining backwards compatibility.
simplify RiPopover trigger rendering with standalone prop, so the trigger can be any custom element that can receive `ref` and pass to it DOM element
---
 .../src/components/base/popover/RiPopover.tsx | 28 +++++++++++--------
 .../ui/src/components/base/popover/types.ts   |  6 ++--
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/redisinsight/ui/src/components/base/popover/RiPopover.tsx b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
index 31ba60b752..cae18ba226 100644
--- a/redisinsight/ui/src/components/base/popover/RiPopover.tsx
+++ b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
@@ -18,6 +18,7 @@ export const RiPopover = ({
   panelClassName,
   className,
   maxWidth = '100%',
+  standalone = false,
   ...props
 }: RiPopoverProps) => {
   // Warn if both button and trigger are provided
@@ -41,15 +42,20 @@ export const RiPopover = ({
   const activeClassName = className ?? panelClassName
 
   // Render trigger element
-  // For new API (trigger): React elements render directly, scalars wrap in span
-  // For old API (button): Always wrap in span (backwards compatibility)
-  const triggerElement =
-    trigger !== undefined && React.isValidElement(activeTrigger) ? (
-      activeTrigger
-    ) : (
-      <span className={anchorClassName}>{activeTrigger}</span>
-    )
+  // If standalone is true, the trigger will be standalone and will not be wrapped in a span
+  // for this to work properly, ether base trigger element is `div`, `span` etc. (base dom element)
+  // or a component that forwards ref
+  const triggerElement = standalone ? (
+    activeTrigger
+  ) : (
+    <span className={anchorClassName}>{activeTrigger}</span>
+  )
 
+  const placement =
+    anchorPosition && anchorPositionMap[anchorPosition]?.placement
+  const align = anchorPosition && anchorPositionMap[anchorPosition]?.align
+  // TODO: maybe use wrapped popover instead of inline style?!
+  const padding = panelPaddingSize && panelPaddingSizeMap[panelPaddingSize]
   return (
     <Popover
       {...props}
@@ -66,11 +72,11 @@ export const RiPopover = ({
       className={activeClassName}
       maxWidth={maxWidth}
       style={{
-        padding: panelPaddingSize && panelPaddingSizeMap[panelPaddingSize],
+        padding,
       }}
       autoFocus={ownFocus}
-      placement={anchorPosition && anchorPositionMap[anchorPosition]?.placement}
-      align={anchorPosition && anchorPositionMap[anchorPosition]?.align}
+      placement={placement}
+      align={align}
     >
       {triggerElement}
     </Popover>
diff --git a/redisinsight/ui/src/components/base/popover/types.ts b/redisinsight/ui/src/components/base/popover/types.ts
index da361ef10c..c6f29619a5 100644
--- a/redisinsight/ui/src/components/base/popover/types.ts
+++ b/redisinsight/ui/src/components/base/popover/types.ts
@@ -27,9 +27,11 @@ export type RiPopoverProps = Omit<
   anchorPosition?: AnchorPosition
   panelPaddingSize?: PanelPaddingSize
   anchorClassName?: string
-  /* @deprecated - use @see{className} - this is popover content wrapper class name */
+  /* @deprecated - use @see {@link className} - this is popover content wrapper class name */
   panelClassName?: string
-  // new preferred prop for popover content wrapper class name (optional)
+  /* new preferred prop for popover content wrapper class name (optional) */
   className?: string
   'data-testid'?: string
+  /* if true, the trigger will be standalone and will not be wrapped in a span */
+  standalone?: boolean
 }

From 7179285405c499cd3a3d0d285684f0c391bb804f Mon Sep 17 00:00:00 2001
From: pd-redis <petar.dzhambazov@redis.com>
Date: Mon, 24 Nov 2025 22:40:37 +0200
Subject: [PATCH 3/3] test: add comprehensive tests for RiPopover and fix
 standalone scalar handling

Add test suite for RiPopover covering all props and behaviors. Fix standalone prop to wrap scalar values in span for asChild compatibility. Improve code readability.
---
 .../base/popover/RiPopover.spec.tsx           | 289 ++++++++++++++++++
 .../src/components/base/popover/RiPopover.tsx |  26 +-
 .../ui/src/components/base/popover/types.ts   |  10 +-
 3 files changed, 312 insertions(+), 13 deletions(-)
 create mode 100644 redisinsight/ui/src/components/base/popover/RiPopover.spec.tsx

diff --git a/redisinsight/ui/src/components/base/popover/RiPopover.spec.tsx b/redisinsight/ui/src/components/base/popover/RiPopover.spec.tsx
new file mode 100644
index 0000000000..b4fa03138d
--- /dev/null
+++ b/redisinsight/ui/src/components/base/popover/RiPopover.spec.tsx
@@ -0,0 +1,289 @@
+import React from 'react'
+import { render, waitForRiPopoverVisible, screen } from 'uiSrc/utils/test-utils'
+import { RiPopover } from './RiPopover'
+import { RiPopoverProps } from './types'
+
+const TestButton = () => (
+  <button type="button" data-testid="popover-trigger">
+    Click me
+  </button>
+)
+
+const renderPopover = (overrides: Partial<RiPopoverProps> = {}) => {
+  return render(
+    <RiPopover
+      button={<TestButton />}
+      isOpen={false}
+      closePopover={jest.fn()}
+      {...overrides}
+    >
+      <div data-testid="popover-content">Popover content</div>
+    </RiPopover>,
+  )
+}
+
+describe('RiPopover', () => {
+  it('should render', () => {
+    expect(renderPopover()).toBeTruthy()
+  })
+
+  it('should render trigger button', () => {
+    renderPopover()
+
+    expect(screen.getByTestId('popover-trigger')).toBeInTheDocument()
+  })
+
+  it('should render popover content when isOpen is true', async () => {
+    renderPopover({ isOpen: true })
+
+    await waitForRiPopoverVisible()
+
+    expect(screen.getByTestId('popover-content')).toBeInTheDocument()
+  })
+
+  it('should not render popover content when isOpen is false', () => {
+    renderPopover({ isOpen: false })
+
+    expect(screen.queryByTestId('popover-content')).not.toBeInTheDocument()
+  })
+
+  describe('button prop (legacy)', () => {
+    it('should wrap button in span by default', () => {
+      renderPopover()
+
+      const trigger = screen.getByTestId('popover-trigger')
+      const wrapper = trigger.parentElement
+
+      expect(wrapper?.tagName).toBe('SPAN')
+    })
+
+    it('should apply anchorClassName to wrapper span', () => {
+      renderPopover({ anchorClassName: 'custom-anchor-class' })
+
+      const trigger = screen.getByTestId('popover-trigger')
+      const wrapper = trigger.parentElement
+
+      expect(wrapper).toHaveClass('custom-anchor-class')
+    })
+  })
+
+  describe('trigger prop (new)', () => {
+    it('should use trigger when provided', () => {
+      renderPopover({
+        trigger: <button data-testid="new-trigger">New Trigger</button>,
+      })
+
+      expect(screen.getByTestId('new-trigger')).toBeInTheDocument()
+      expect(screen.queryByTestId('popover-trigger')).not.toBeInTheDocument()
+    })
+
+    it('should wrap trigger in span by default (standalone=false)', () => {
+      renderPopover({
+        trigger: <button data-testid="new-trigger">New Trigger</button>,
+      })
+
+      const trigger = screen.getByTestId('new-trigger')
+      const wrapper = trigger.parentElement
+
+      expect(wrapper?.tagName).toBe('SPAN')
+    })
+
+    it('should render trigger directly when standalone is true', () => {
+      renderPopover({
+        trigger: <div data-testid="standalone-trigger">Standalone</div>,
+        standalone: true,
+      })
+
+      const trigger = screen.getByTestId('standalone-trigger')
+      const wrapper = trigger.parentElement
+
+      // Should not be wrapped in span
+      expect(wrapper?.tagName).not.toBe('SPAN')
+      expect(wrapper?.tagName).toBe('DIV')
+    })
+
+    it('should apply anchorClassName to wrapper when standalone is false', () => {
+      renderPopover({
+        trigger: <button data-testid="new-trigger">New Trigger</button>,
+        anchorClassName: 'custom-anchor-class',
+      })
+
+      const trigger = screen.getByTestId('new-trigger')
+      const wrapper = trigger.parentElement
+
+      expect(wrapper).toHaveClass('custom-anchor-class')
+    })
+
+    it('should not apply anchorClassName when standalone is true', () => {
+      renderPopover({
+        trigger: <div data-testid="standalone-trigger">Standalone</div>,
+        standalone: true,
+        anchorClassName: 'custom-anchor-class',
+      })
+
+      const trigger = screen.getByTestId('standalone-trigger')
+
+      // anchorClassName should not be applied since there's no wrapper
+      expect(trigger).not.toHaveClass('custom-anchor-class')
+    })
+  })
+
+  describe('prop conflicts and warnings', () => {
+    it('should warn when both button and trigger are provided', () => {
+      const consoleWarnSpy = jest.spyOn(console, 'warn')
+
+      renderPopover({ trigger: <button>Trigger</button> })
+
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        "[RiPopover]: Both 'button' and 'trigger' props are provided. Using 'trigger'. Please migrate to 'trigger' prop.",
+      )
+    })
+
+    it('should warn when both panelClassName and className are provided', () => {
+      const consoleWarnSpy = jest.spyOn(console, 'warn')
+
+      renderPopover({
+        panelClassName: 'old-class',
+        className: 'new-class',
+      })
+
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        "[RiPopover]: Both 'panelClassName' and 'className' props are provided. Using 'className'. Please migrate to 'className' prop.",
+      )
+    })
+  })
+
+  describe('className prop', () => {
+    it('should use className when provided', async () => {
+      renderPopover({
+        isOpen: true,
+        className: 'custom-class',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = screen.queryByRole('dialog')
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveClass('custom-class')
+    })
+
+    it('should fall back to panelClassName when className is not provided', async () => {
+      renderPopover({
+        isOpen: true,
+        panelClassName: 'fallback-class',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = screen.queryByRole('dialog')
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveClass('fallback-class')
+    })
+
+    it('should prefer className over panelClassName when both are provided', async () => {
+      renderPopover({
+        isOpen: true,
+        panelClassName: 'old-class',
+        className: 'new-class',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = screen.queryByRole('dialog')
+
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveClass('new-class')
+      expect(popover).not.toHaveClass('old-class')
+    })
+  })
+
+  describe('panelPaddingSize', () => {
+    it('should apply padding style based on panelPaddingSize', async () => {
+      renderPopover({
+        isOpen: true,
+        panelPaddingSize: 'm',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = screen.queryByRole('dialog')
+
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveStyle({ padding: '18px' })
+    })
+
+    it('should apply no padding when panelPaddingSize is none', async () => {
+      renderPopover({
+        isOpen: true,
+        panelPaddingSize: 'none',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = screen.queryByRole('dialog')
+
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveStyle({ padding: '0px' })
+    })
+  })
+
+  describe('scalar trigger values', () => {
+    it('should wrap string trigger in span', () => {
+      const { container } = renderPopover({ trigger: 'String trigger' })
+
+      const text = screen.getByText('String trigger')
+      // The Popover component might wrap our span in a div, so check if span exists
+      const span = container.querySelector('span')
+      expect(span).toBeInTheDocument()
+      expect(span).toContainElement(text)
+    })
+
+    it('should wrap number trigger in span', () => {
+      const { container } = renderPopover({ trigger: 123 })
+
+      const text = screen.getByText('123')
+      // The Popover component might wrap our span in a div, so check if span exists
+      const span = container.querySelector('span')
+      expect(span).toBeInTheDocument()
+      expect(span).toContainElement(text)
+    })
+
+    it('should wrap scalar trigger in span when standalone is true', () => {
+      // When standalone is true and trigger is a scalar (string, number, etc.),
+      // we wrap it in a span because RadixPopover.Trigger with asChild requires a React element
+      const { container } = renderPopover({
+        trigger: 'String trigger',
+        standalone: true,
+      })
+
+      const text = screen.getByText('String trigger')
+      // Should be wrapped in a span (without anchorClassName)
+      const span = container.querySelector('span')
+      expect(span).toBeInTheDocument()
+      expect(span).toContainElement(text)
+      // The span should not have anchorClassName when standalone is true
+      expect(span).not.toHaveClass()
+    })
+  })
+
+  describe('backwards compatibility', () => {
+    it('should work with button prop only (legacy behavior)', () => {
+      renderPopover()
+
+      expect(screen.getByTestId('popover-trigger')).toBeInTheDocument()
+    })
+
+    it('should work with panelClassName prop only (legacy behavior)', async () => {
+      const { getByRole } = renderPopover({
+        isOpen: true,
+        panelClassName: 'legacy-class',
+      })
+
+      await waitForRiPopoverVisible()
+
+      const popover = getByRole('dialog')
+      expect(popover).toBeInTheDocument()
+      expect(popover).toHaveClass('legacy-class')
+    })
+  })
+})
diff --git a/redisinsight/ui/src/components/base/popover/RiPopover.tsx b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
index cae18ba226..4bb83bec02 100644
--- a/redisinsight/ui/src/components/base/popover/RiPopover.tsx
+++ b/redisinsight/ui/src/components/base/popover/RiPopover.tsx
@@ -24,14 +24,14 @@ export const RiPopover = ({
   // Warn if both button and trigger are provided
   if (button !== undefined && trigger !== undefined) {
     console.warn(
-      "RiPopover: Both 'button' and 'trigger' props are provided. Using 'trigger'. Please migrate to 'trigger' prop.",
+      "[RiPopover]: Both 'button' and 'trigger' props are provided. Using 'trigger'. Please migrate to 'trigger' prop.",
     )
   }
 
   // Warn if both panelClassName and className are provided
   if (panelClassName !== undefined && className !== undefined) {
     console.warn(
-      "RiPopover: Both 'panelClassName' and 'className' props are provided. Using 'className'. Please migrate to 'className' prop.",
+      "[RiPopover]: Both 'panelClassName' and 'className' props are provided. Using 'className'. Please migrate to 'className' prop.",
     )
   }
 
@@ -43,13 +43,23 @@ export const RiPopover = ({
 
   // Render trigger element
   // If standalone is true, the trigger will be standalone and will not be wrapped in a span
-  // for this to work properly, ether base trigger element is `div`, `span` etc. (base dom element)
+  // for this to work properly, either base trigger element is `div`, `span` etc. (base dom element)
   // or a component that forwards ref
-  const triggerElement = standalone ? (
-    activeTrigger
-  ) : (
-    <span className={anchorClassName}>{activeTrigger}</span>
-  )
+  // However, if standalone is true and trigger is a scalar (string, number, etc.),
+  // we need to wrap it in a span because RadixPopover.Trigger with asChild requires a React element
+  let triggerElement: React.ReactNode
+
+  if (standalone) {
+    if (React.isValidElement(activeTrigger)) {
+      triggerElement = activeTrigger
+    } else {
+      // Wrap scalar values in span for asChild compatibility
+      triggerElement = <span>{activeTrigger}</span>
+    }
+  } else {
+    // Always wrap in span with anchorClassName for backwards compatibility
+    triggerElement = <span className={anchorClassName}>{activeTrigger}</span>
+  }
 
   const placement =
     anchorPosition && anchorPositionMap[anchorPosition]?.placement
diff --git a/redisinsight/ui/src/components/base/popover/types.ts b/redisinsight/ui/src/components/base/popover/types.ts
index c6f29619a5..919a84bd89 100644
--- a/redisinsight/ui/src/components/base/popover/types.ts
+++ b/redisinsight/ui/src/components/base/popover/types.ts
@@ -20,18 +20,18 @@ export type RiPopoverProps = Omit<
   isOpen?: PopoverProps['open']
   closePopover?: PopoverProps['onClickOutside']
   ownFocus?: PopoverProps['autoFocus']
-  /* @deprecated old prop for popover trigger element, use trigger */
+  /** @deprecated old prop for popover trigger element, use {@linkcode trigger} */
   button?: PopoverProps['content']
-  /* preferred prop for popover trigger element (optional) */
+  /** preferred prop for popover trigger element (optional) */
   trigger?: ReactNode
   anchorPosition?: AnchorPosition
   panelPaddingSize?: PanelPaddingSize
   anchorClassName?: string
-  /* @deprecated - use @see {@link className} - this is popover content wrapper class name */
+  /** @deprecated - use {@linkcode className} - this is popover content wrapper class name */
   panelClassName?: string
-  /* new preferred prop for popover content wrapper class name (optional) */
+  /** new preferred prop for popover content wrapper class name (optional) */
   className?: string
   'data-testid'?: string
-  /* if true, the trigger will be standalone and will not be wrapped in a span */
+  /** if true, the trigger will be standalone and will not be wrapped in a span */
   standalone?: boolean
 }