Merge pull request #2273 from adevinta/refactor-popover-has-button

feat(popover): use css "has" selector rather than JS to apply specific styles
adevinta · Jun 25, 2024 · add0d62 · add0d62
2 parents ee3b26b + 293fe01
commit add0d62
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 28 deletions.
diff --git a/packages/components/popover/src/PopoverCloseButton.tsx b/packages/components/popover/src/PopoverCloseButton.tsx
@@ -3,24 +3,14 @@ import { Icon } from '@spark-ui/icon'
 import { IconButton } from '@spark-ui/icon-button'
 import { Close as CloseSVG } from '@spark-ui/icons/dist/icons/Close'
 import { cx } from 'class-variance-authority'
-import { forwardRef, useLayoutEffect } from 'react'
-
-import { usePopover } from './PopoverContext'
+import { forwardRef } from 'react'
 
 export type CloseButtonProps = RadixPopover.PopoverCloseProps & {
   'aria-label': string
 }
 
 export const CloseButton = forwardRef<HTMLButtonElement, CloseButtonProps>(
   ({ 'aria-label': ariaLabel, className, ...rest }, ref) => {
-    const { setHasCloseButton } = usePopover()
-
-    useLayoutEffect(() => {
-      setHasCloseButton(true)
-
-      return () => setHasCloseButton(false)
-    }, [setHasCloseButton])
-
     return (
       <RadixPopover.Close
         data-spark-component="popover-close-button"

diff --git a/packages/components/popover/src/PopoverContent.styles.ts b/packages/components/popover/src/PopoverContent.styles.ts
@@ -27,12 +27,7 @@ export const styles = cva(
       enforceBoundaries: {
         true: ['max-w-[--radix-popper-available-width]'],
       },
-      /**
-       * When there is a close button, padding to the right side must be adjusted to avoid content overlapping with it.
-       */
-      hasCloseButton: {
-        true: 'pr-[40px]',
-      },
+
       inset: {
         true: 'overflow-hidden',
         false: 'p-lg',
@@ -44,9 +39,11 @@ export const styles = cva(
     },
     compoundVariants: [
       {
-        hasCloseButton: true,
-        inset: true,
-        class: 'pr-none',
+        inset: false,
+        /**
+         * When there is a close button, padding to the right side must be adjusted to avoid content overlapping with it.
+         */
+        class: 'has-[[data-spark-component=popover-close-button]]:pr-3xl',
       },
       {
         enforceBoundaries: false,
@@ -57,7 +54,6 @@ export const styles = cva(
     defaultVariants: {
       matchTriggerWidth: false,
       enforceBoundaries: false,
-      hasCloseButton: false,
       inset: false,
       intent: 'surface',
       elevation: 'popover',

diff --git a/packages/components/popover/src/PopoverContent.tsx b/packages/components/popover/src/PopoverContent.tsx
@@ -31,15 +31,14 @@ export const Content = forwardRef<HTMLDivElement, ContentProps>(
     },
     ref
   ) => {
-    const { hasCloseButton, headerId, intent } = usePopover()
+    const { headerId, intent } = usePopover()
 
     return (
       <RadixPopover.Content
         aria-labelledby={headerId || ariaLabelledBy}
         className={styles({
           enforceBoundaries: !!collisionBoundary,
           matchTriggerWidth,
-          hasCloseButton,
           inset,
           elevation,
           intent,

diff --git a/packages/components/popover/src/PopoverContext.tsx b/packages/components/popover/src/PopoverContext.tsx
@@ -14,8 +14,6 @@ export type PopoverIntent =
   | 'info'
   | 'neutral'
 export interface PopoverContextState {
-  hasCloseButton: boolean
-  setHasCloseButton: (value: boolean) => void
   headerId: HeaderId
   setHeaderId: (id: HeaderId) => void
   intent: PopoverIntent
@@ -32,14 +30,11 @@ export const PopoverProvider = ({
   children: ReactNode
   intent: PopoverIntent
 }) => {
-  const [hasCloseButton, setHasCloseButton] = useState(false)
   const [headerId, setHeaderId] = useState<HeaderId>(null)
 
   return (
     <PopoverContext.Provider
       value={{
-        hasCloseButton,
-        setHasCloseButton,
         headerId,
         setHeaderId,
         intent,