From 1f52defc002b47b67da2b788dcd23fcc2042046e Mon Sep 17 00:00:00 2001 From: Rohan Chakraborty Date: Wed, 4 Feb 2026 03:43:51 +0530 Subject: [PATCH] feat: migrate popover to base ui --- .../content/docs/components/popover/demo.ts | 2 +- .../content/docs/components/popover/index.mdx | 7 -- .../content/docs/components/popover/props.ts | 45 +++++++---- .../popover/__tests__/popover.test.tsx | 51 +++++++------ .../components/popover/popover.module.css | 6 +- .../raystack/components/popover/popover.tsx | 75 ++++++++++--------- 6 files changed, 100 insertions(+), 86 deletions(-) diff --git a/apps/www/src/content/docs/components/popover/demo.ts b/apps/www/src/content/docs/components/popover/demo.ts index 3ae740595..0a64da62e 100644 --- a/apps/www/src/content/docs/components/popover/demo.ts +++ b/apps/www/src/content/docs/components/popover/demo.ts @@ -7,7 +7,7 @@ export const getCode = (props: any) => { return ` - + ${children} diff --git a/apps/www/src/content/docs/components/popover/index.mdx b/apps/www/src/content/docs/components/popover/index.mdx index e1d4bdf50..c359e4c0b 100644 --- a/apps/www/src/content/docs/components/popover/index.mdx +++ b/apps/www/src/content/docs/components/popover/index.mdx @@ -43,10 +43,3 @@ Control the position and alignment of your popover relative to its trigger. Customize how the popover aligns with its trigger. - -## Accessibility - -The Callout component includes appropriate ARIA attributes for accessibility: - -- Uses semantic HTML elements for proper structure -- Dismiss button includes `aria-label` for screen readers diff --git a/apps/www/src/content/docs/components/popover/props.ts b/apps/www/src/content/docs/components/popover/props.ts index 646dd4d70..4887dfffa 100644 --- a/apps/www/src/content/docs/components/popover/props.ts +++ b/apps/www/src/content/docs/components/popover/props.ts @@ -13,32 +13,49 @@ export interface PopoverRootProps { } export interface PopoverContentProps { - /** - * Accessible label for the popover content. - * @default "Popover content" - */ - ariaLabel?: string; - /** Preferred side of the trigger to render. */ side?: 'top' | 'right' | 'bottom' | 'left'; - /** Distance in pixels from the trigger. */ - sideOffset?: number; - /** Alignment relative to trigger. */ align?: 'start' | 'center' | 'end'; + /** Distance in pixels from the trigger. */ + sideOffset?: number; + /** Offset in pixels from alignment edge. */ alignOffset?: number; - /** Boolean to prevent collision with viewport edges. */ - avoidCollisions?: boolean; - /** Padding between content and viewport edges. */ collisionPadding?: number; + + /** Boundary element for collision detection. */ + collisionBoundary?: Element | Element[] | null; + + /** Additional CSS class name. */ + className?: string; + + /** Additional inline styles. */ + style?: React.CSSProperties; + + /** Custom render function. + * + * @remarks `ReactElement | function` + */ + render?: + | React.ReactElement + | ((props: any, state: any) => React.ReactElement); + + /** Element to receive initial focus when popover opens. */ + initialFocus?: boolean | number | React.RefObject; + + /** Element to receive focus when popover closes. */ + finalFocus?: boolean | React.RefObject; + + /** Content to render inside the popover. */ + children?: React.ReactNode; } export interface PopoverTriggerProps { - /** Boolean to merge props onto child element. */ - asChild?: boolean; + /** Additional CSS class name. */ + className?: string; } diff --git a/packages/raystack/components/popover/__tests__/popover.test.tsx b/packages/raystack/components/popover/__tests__/popover.test.tsx index 2c138ab4b..e0a43f19a 100644 --- a/packages/raystack/components/popover/__tests__/popover.test.tsx +++ b/packages/raystack/components/popover/__tests__/popover.test.tsx @@ -1,6 +1,6 @@ +import { Popover as PopoverPrimitive } from '@base-ui/react'; import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { Popover as PopoverPrimitive } from 'radix-ui'; import React from 'react'; import { describe, expect, it, vi } from 'vitest'; import { Button } from '~/components/button'; @@ -13,9 +13,9 @@ const POPOVER_CONTENT = 'This is popover content'; const BasicPopover = ({ children = {POPOVER_CONTENT}, ...props -}: PopoverPrimitive.PopoverProps) => ( +}: PopoverPrimitive.Root.Props) => ( - + {children} @@ -96,13 +96,18 @@ describe('Popover', () => { ); const content = screen.getByRole('dialog'); - expect(content).toHaveAttribute('data-align', align); + // Base UI uses data-align on the positioner, not the popup + const positioner = content.closest('[data-align]'); + expect(positioner).toHaveAttribute('data-align', align); }); it('applies default align to center', async () => { await renderAndOpenPopover(); - const dialog = screen.getByRole('dialog'); - expect(dialog).toHaveAttribute('data-align', 'center'); + await waitFor(() => { + const dialog = screen.getByRole('dialog'); + const positioner = dialog.closest('[data-align]'); + expect(positioner).toHaveAttribute('data-align', 'center'); + }); }); const sideValues = ['top', 'right', 'bottom', 'left'] as const; @@ -113,13 +118,18 @@ describe('Popover', () => { ); const content = screen.getByRole('dialog'); - expect(content).toHaveAttribute('data-side', side); + // Base UI uses data-side on the positioner, not the popup + const positioner = content.closest('[data-side]'); + expect(positioner).toHaveAttribute('data-side', side); }); it('applies default side to bottom', async () => { await renderAndOpenPopover(); - const dialog = screen.getByRole('dialog'); - expect(dialog).toHaveAttribute('data-side', 'bottom'); + await waitFor(() => { + const dialog = screen.getByRole('dialog'); + const positioner = dialog.closest('[data-side]'); + expect(positioner).toHaveAttribute('data-side', 'bottom'); + }); }); }); @@ -180,7 +190,10 @@ describe('Popover', () => { const trigger = screen.getByText(TRIGGER_TEXT); await user.click(trigger); - expect(onOpenChange).toHaveBeenCalledWith(true); + expect(onOpenChange).toHaveBeenCalled(); + // Base UI passes the open state as the first argument + const callArgs = onOpenChange.mock.calls[0]; + expect(callArgs[0]).toBe(true); }); }); @@ -191,32 +204,18 @@ describe('Popover', () => { await waitFor(() => { const dialog = screen.getByRole('dialog'); expect(dialog).toBeInTheDocument(); - expect(dialog).toHaveAttribute('aria-modal', 'true'); }); }); - it('has default ARIA label', async () => { + it('has proper ARIA attributes', async () => { await renderAndOpenPopover(); await waitFor(() => { const dialog = screen.getByRole('dialog'); - expect(dialog).toHaveAttribute('aria-label', 'Popover content'); + expect(dialog).toBeInTheDocument(); }); }); - it('uses custom ARIA label when provided', async () => { - await renderAndOpenPopover( - - - {POPOVER_CONTENT} - - - ); - - const dialog = screen.getByRole('dialog'); - expect(dialog).toHaveAttribute('aria-label', 'Custom popover label'); - }); - it('has proper focus management', async () => { await renderAndOpenPopover( diff --git a/packages/raystack/components/popover/popover.module.css b/packages/raystack/components/popover/popover.module.css index 46676e3c8..8da7a13ee 100644 --- a/packages/raystack/components/popover/popover.module.css +++ b/packages/raystack/components/popover/popover.module.css @@ -1,15 +1,15 @@ -.popover { +.popoverPositioner { z-index: var(--rs-z-index-portal); +} +.popover { outline: 0; overflow: hidden; font-size: var(--rs-font-size-small); line-height: var(--rs-line-height-small); letter-spacing: var(--rs-letter-spacing-small); - box-sizing: border-box; min-width: 120px; max-width: 18rem; - padding: var(--rs-space-3); background-color: var(--rs-color-background-base-primary); border-radius: var(--rs-radius-2); diff --git a/packages/raystack/components/popover/popover.tsx b/packages/raystack/components/popover/popover.tsx index 554c9f23e..823aca06c 100644 --- a/packages/raystack/components/popover/popover.tsx +++ b/packages/raystack/components/popover/popover.tsx @@ -1,52 +1,57 @@ 'use client'; -import { cva } from 'class-variance-authority'; -import { Popover as PopoverPrimitive } from 'radix-ui'; -import React from 'react'; - +import { Popover as PopoverPrimitive } from '@base-ui/react'; +import { cx } from 'class-variance-authority'; +import { ElementRef, forwardRef } from 'react'; import styles from './popover.module.css'; -const popoverContent = cva(styles.popover); - export interface PopoverContentProps - extends React.ComponentPropsWithoutRef { - ariaLabel?: string; -} + extends Omit< + PopoverPrimitive.Positioner.Props, + 'render' | 'className' | 'style' + >, + PopoverPrimitive.Popup.Props {} -const PopoverContent = React.forwardRef< - React.ElementRef, +const PopoverContent = forwardRef< + ElementRef, PopoverContentProps >( ( { + initialFocus, + finalFocus, className, - align = 'center', - sideOffset = 4, - ariaLabel = 'Popover content', - collisionPadding = 3, - ...props + style, + render, + children, + ...positionerProps }, ref - ) => ( - - - {props.children} - - - ) + ) => { + return ( + + + + {children} + + + + ); + } ); -PopoverContent.displayName = PopoverPrimitive.Content.displayName; +PopoverContent.displayName = 'Popover.Content'; export const Popover = Object.assign(PopoverPrimitive.Root, { Trigger: PopoverPrimitive.Trigger,