HoverCard Component #583
HoverCard Component #583
Conversation
WalkthroughA new Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #583 +/- ##
==========================================
+ Coverage 94.00% 94.11% +0.11%
==========================================
Files 14 14
Lines 100 102 +2
Branches 27 28 +1
==========================================
+ Hits 94 96 +2
Misses 6 6 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (3)
src/components/ui/HoverCard/fragments/HoverCardPortal.tsx (1)
1-7: Consider adding TypeScript typesSince this is a TypeScript file, we should properly type the props and the component.
-import React from 'react'; +import { createPortal } from 'react-dom'; + +interface HoverCardPortalProps extends React.HTMLAttributes<HTMLDivElement> { + children: React.ReactNode; +} -const HoverCardPortal = ({ children, ...props }) => { +const HoverCardPortal = ({ children, ...props }: HoverCardPortalProps): React.ReactPortal => {src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx (1)
7-7: Add JSDoc documentation for the componentThe component is missing proper documentation for its usage and props.
Add JSDoc documentation:
+/** + * HoverCardTrigger component + * + * Trigger element that controls the visibility of a hover card. + * Must be used within a HoverCard component. + * + * @example + * ```tsx + * <HoverCard> + * <HoverCardTrigger>Hover me</HoverCardTrigger> + * <HoverCardContent>Hover card content</HoverCardContent> + * </HoverCard> + * ``` + */ export default HoverCardTrigger;src/components/ui/HoverCard/HoverCard.tsx (1)
24-24: Consider adding component documentationThe export looks good, but since this is a new component, consider adding JSDoc documentation to describe its purpose, props, and usage examples.
Add documentation above the component:
+/** + * HoverCard component displays content when hovering over a trigger element. + * + * @example + * ```tsx + * <HoverCard + * trigger={<button>Hover me</button>} + * content={<div>Hover card content</div>} + * /> + * ``` + */ export default HoverCard;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
src/components/ui/HoverCard/HoverCard.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardArrow.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardContent.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardPortal.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardRoot.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx(1 hunks)src/components/ui/HoverCard/stories/HoverCard.stories.js(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/components/ui/HoverCard/fragments/HoverCardArrow.tsx
- src/components/ui/HoverCard/fragments/HoverCardContent.tsx
- src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
- src/components/ui/HoverCard/stories/HoverCard.stories.js
🔇 Additional comments (2)
src/components/ui/HoverCard/fragments/HoverCardPortal.tsx (1)
1-7: Verify HoverCard component integration
Let's verify how this component is being used in the parent HoverCard component and related stories.
✅ Verification successful
HoverCardPortal implementation appears safe to modify
The verification shows that HoverCardPortal is a simple wrapper component that's only used within the HoverCard component itself. The implementation is basic and matches the pattern of other HoverCard fragments (Root, Content) which are also simple div wrappers. No external usages or complex portal implementations were found that could be impacted by this change.
- The component is only imported in
HoverCard.tsx - No stories or tests were found that could be affected
- The implementation aligns with other fragment components in the same directory
- No external usages of the component were discovered
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check HoverCard usage patterns
echo "Checking HoverCard component usage..."
rg -l "HoverCardPortal" --type ts --type tsx
# Check for potential z-index related styles
echo "Checking for z-index styles that might affect the portal..."
rg "z-index" --type css --type scss
Length of output: 351
Script:
#!/bin/bash
# Search for HoverCardPortal usage with correct file extensions
echo "Checking HoverCard component usage..."
fd -e ts -e tsx -x rg -l "HoverCardPortal" {}
# Check for stories and tests
echo "Checking stories and tests..."
fd -e stories.tsx -e test.tsx -e spec.tsx -x rg -l "HoverCard" {}
# Check for the parent HoverCard component
echo "Checking HoverCard component implementation..."
fd "HoverCard.tsx" -x cat {}
Length of output: 1267
Script:
#!/bin/bash
# Check HoverCardRoot implementation to understand the portal context
echo "Checking HoverCardRoot implementation..."
cat ./src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
# Check HoverCardContent implementation
echo "Checking HoverCardContent implementation..."
cat ./src/components/ui/HoverCard/fragments/HoverCardContent.tsx
# Look for actual usage of HoverCard component in the codebase
echo "Checking HoverCard usage in the codebase..."
rg -l "import.*HoverCard" --type ts --type tsx | grep -v "HoverCard/"
Length of output: 837
Script:
#!/bin/bash
# Look for actual usage of HoverCard component with correct extension handling
echo "Checking HoverCard usage in the codebase..."
fd -e ts -e tsx -x rg -l "import.*HoverCard from" {} | grep -v "HoverCard/"
# Check if there are any stories
echo "Checking for HoverCard stories..."
fd -g "*HoverCard*.stories.tsx" -g "*HoverCard*.story.tsx"
# Check if there are any tests
echo "Checking for HoverCard tests..."
fd -g "*HoverCard*.test.tsx" -g "*HoverCard*.spec.tsx"
Length of output: 659
src/components/ui/HoverCard/HoverCard.tsx (1)
1-7: LGTM: Imports are well-organized
The imports are clean and well-structured, following a logical grouping with core React first, followed by fragment components.
| const HoverCardTrigger = ({ children, ...props }) => { | ||
| return <button onClick={props.onClick}>{children}</button>; | ||
| }; |
There was a problem hiding this comment.
Add proper hover card trigger accessibility and behavior
The current implementation has several critical issues:
- Missing proper hover/focus event handlers for hover card behavior
- Missing ARIA attributes for accessibility
- Missing keyboard interaction support
Consider this implementation:
-const HoverCardTrigger = ({ children, ...props }) => {
- return <button onClick={props.onClick}>{children}</button>;
+import { forwardRef } from 'react';
+
+const HoverCardTrigger = forwardRef<
+ HTMLButtonElement,
+ React.ButtonHTMLAttributes<HTMLButtonElement>
+>(({ children, ...props }, ref) => {
+ return (
+ <button
+ ref={ref}
+ type="button"
+ aria-haspopup="dialog"
+ aria-expanded={props['aria-expanded']}
+ onMouseEnter={props.onMouseEnter}
+ onMouseLeave={props.onMouseLeave}
+ onFocus={props.onFocus}
+ onBlur={props.onBlur}
+ {...props}
+ >
+ {children}
+ </button>
+ );
+});
+
+HoverCardTrigger.displayName = 'HoverCardTrigger';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const HoverCardTrigger = ({ children, ...props }) => { | |
| return <button onClick={props.onClick}>{children}</button>; | |
| }; | |
| import { forwardRef } from 'react'; | |
| const HoverCardTrigger = forwardRef< | |
| HTMLButtonElement, | |
| React.ButtonHTMLAttributes<HTMLButtonElement> | |
| >(({ children, ...props }, ref) => { | |
| return ( | |
| <button | |
| ref={ref} | |
| type="button" | |
| aria-haspopup="dialog" | |
| aria-expanded={props['aria-expanded']} | |
| onMouseEnter={props.onMouseEnter} | |
| onMouseLeave={props.onMouseLeave} | |
| onFocus={props.onFocus} | |
| onBlur={props.onBlur} | |
| {...props} | |
| > | |
| {children} | |
| </button> | |
| ); | |
| }); | |
| HoverCardTrigger.displayName = 'HoverCardTrigger'; |
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (14)
src/components/ui/HoverCard/contexts/HoverCardContext.tsx (3)
1-1: Use named imports for better tree-shakingConsider using named imports from React for better tree-shaking support.
-import { createContext } from 'react'; +import * as React from 'react';
3-6: Enhance type definition with readonly modifier and documentationThe type definition could be improved with better documentation and type safety.
+/** + * Context type for managing hover card state + */ type HoverCardContextType = { - isOpen: boolean; - handleOpenChange: (open: boolean) => void; + readonly isOpen: boolean; + readonly handleOpenChange: (open: boolean) => void; };
13-13: Use named exports for better maintainabilityConsider using named exports instead of default exports for better maintainability and refactoring support.
-export default HoverCardContext; +export { HoverCardContext, useHoverCard };src/components/ui/HoverCard/fragments/HoverCardArrow.tsx (2)
7-7: Add error handling for missing context.The component assumes the context will always be available, which might not be true if used outside of a HoverCard.
Consider adding a check:
-const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext); +const context = useContext(HoverCardContext); +if (!context) { + throw new Error('HoverCardArrow must be used within a HoverCard component'); +} +const { floatingContext, arrowRef, rootClass } = context;
1-12: Consider adding tests and documentation.As this is a crucial UI component that handles positioning and rendering:
- Add unit tests to verify proper context consumption and prop forwarding
- Add documentation for:
- Component usage examples
- Available props
- Context requirements
- Accessibility considerations
- Consider adding aria-hidden attribute since this is a decorative arrow element
Would you like me to help create a test suite or documentation template for this component?
src/core/primitives/Floater/index.tsx (1)
1-1: Consider adding type imports for better type safetyWhile the functional imports look good, consider importing the TypeScript types for these utilities to ensure type safety throughout the HoverCard implementation.
-import { FloatingOverlay, FloatingPortal, FloatingFocusManager, useFloating, FloatingArrow, arrow, useRole, useInteractions, useDismiss, useHover, flip, shift, hide, offset } from '@floating-ui/react'; +import type { Placement, FloatingContext, ReferenceType, FloatingEvents } from '@floating-ui/react'; +import { FloatingOverlay, FloatingPortal, FloatingFocusManager, useFloating, FloatingArrow, arrow, useRole, useInteractions, useDismiss, useHover, flip, shift, hide, offset } from '@floating-ui/react';src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx (1)
19-19: Add display name for debuggingAdd a display name to improve component identification in React DevTools.
Add this before the export:
+HoverCardTrigger.displayName = 'HoverCardTrigger'; export default HoverCardTrigger;src/components/ui/HoverCard/fragments/HoverCardContent.tsx (1)
1-3: Add TypeScript types for better type safetyConsider adding TypeScript interfaces for the component props and properly typing the context usage.
import React, { useContext, useEffect } from 'react'; +import { HTMLProps } from 'react'; import HoverCardContext from '../contexts/HoverCardContext'; + +interface HoverCardContentProps extends HTMLProps<HTMLDivElement> { + children: React.ReactNode; +}src/components/ui/HoverCard/stories/HoverCard.stories.js (3)
9-15: Extract Content component and parameterize text contentThe Content component should be extracted outside the render function for reusability and the text content should be configurable through story args.
+const Content = ({ text = 'The quick brown fox jumps over the lazy dog' }) => ( + <div> + <div className='space-y-2'> + {text} + </div> + </div> +); export default { // ... render: (args) => { - const Content = () => { - return <div> - <div className='space-y-2'> - The quick brown fox jumps over the lazy dog - </div> - </div>; - }; return ( // ... - <HoverCard content={<Content />} {...args}> + <HoverCard content={<Content text={args.text} />} {...args}>
4-22: Add component documentation and props tableThe story is missing documentation about the HoverCard component's props and usage examples.
export default { title: 'Components/HoverCard', component: HoverCard, + parameters: { + docs: { + description: { + component: 'A hover card component that displays additional content when hovering over a trigger element.' + } + } + }, + argTypes: { + content: { + description: 'The content to display in the hover card', + control: 'text' + }, + open: { + description: 'Controls the open state in controlled mode', + control: 'boolean' + } + }, render: (args) => {
24-33: Enhance story variants coverageThe current variants don't showcase different use cases of the HoverCard component. Consider adding variants for:
- Different positions (top, bottom, left, right)
- Custom arrow styling
- Different content sizes
- Loading states
- Error states
export const All = { + args: { + text: 'Basic hover card example' + } }; +export const Positions = { + render: (args) => ( + <div className="grid grid-cols-2 gap-4"> + <HoverCard position="top" content={<Content />}>Top</HoverCard> + <HoverCard position="bottom" content={<Content />}>Bottom</HoverCard> + <HoverCard position="left" content={<Content />}>Left</HoverCard> + <HoverCard position="right" content={<Content />}>Right</HoverCard> + </div> + ) +}; export const Controlled = { args: { open: true } };src/core/primitives/Primitive/index.tsx (2)
14-23: Enhance type safety by parameterizing element typesTo improve type safety, consider parameterizing the
PrimitiveComponentwith the specific element type. This ensures that props and refs are accurately typed according to theelementType.Here's how you might adjust the code:
-const createPrimitiveComponent = (elementType: SupportedElement) => { - const PrimitiveComponent = React.forwardRef<HTMLElement, PrimitiveProps>((props, ref) => { +const createPrimitiveComponent = <T extends SupportedElement>(elementType: T) => { + const PrimitiveComponent = React.forwardRef<React.ElementRef<T>, React.ComponentPropsWithoutRef<T> & PrimitiveProps>((props, ref) => { const { asChild = false, children, ...elementProps } = props; if (asChild && React.isValidElement(children)) { return React.cloneElement(children, { ...elementProps, ref }); } return React.createElement(elementType, { ...elementProps, ref }, children); });This change uses generics to align the component's props and ref with the specific
elementType, enhancing type safety and reducing potential runtime errors.
30-36: Simplify type annotations for maintainabilityWhile explicit type annotations on
Primitiveenhance type safety, they can be complex. If TypeScript can infer the types correctly, consider letting it do so to improve readability.You might simplify the code as follows:
-const Primitive = SUPPORTED_HTML_ELEMENTS.reduce<Record<SupportedElement, React.ForwardRefExoticComponent<PrimitiveProps & React.RefAttributes<HTMLElement>>>>( +const Primitive = SUPPORTED_HTML_ELEMENTS.reduce( (components, elementType) => { components[elementType] = createPrimitiveComponent(elementType); return components; - }, - {} as Record<SupportedElement, React.ForwardRefExoticComponent<PrimitiveProps & React.RefAttributes<HTMLElement>>> + }, + {} as Record<SupportedElement, ReturnType<typeof createPrimitiveComponent>> );This allows TypeScript to handle type inference, making the code cleaner and easier to maintain.
src/components/ui/HoverCard/fragments/HoverCardRoot.tsx (1)
35-41: Clean up commented-out middleware configurationsThere are several commented-out middleware configurations in the
middlewarearray, such asFloater.shiftandFloater.hide. Consider removing these if they are no longer needed, or add comments explaining why they are commented out.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (12)
src/components/tools/Popper/Popper.tsx(1 hunks)src/components/ui/HoverCard/HoverCard.tsx(1 hunks)src/components/ui/HoverCard/contexts/HoverCardContext.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardArrow.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardContent.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardRoot.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx(1 hunks)src/components/ui/HoverCard/stories/HoverCard.stories.js(1 hunks)src/core/primitives/Floater/index.tsx(1 hunks)src/core/primitives/Primitive/index.tsx(1 hunks)styles/themes/components/hover-card.scss(1 hunks)styles/themes/default.scss(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- styles/themes/components/hover-card.scss
- src/components/tools/Popper/Popper.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/HoverCard/HoverCard.tsx
🔇 Additional comments (12)
src/components/ui/HoverCard/contexts/HoverCardContext.tsx (1)
1-13: Consider implementing a HoverCardProvider component
To complete this context implementation, you'll need a provider component that manages the hover card state. This provider should handle:
- Mouse enter/leave events
- Focus management
- Keyboard interactions (Esc to close)
- Delay timing for open/close animations
Let's check for other hover card related files:
src/components/ui/HoverCard/fragments/HoverCardArrow.tsx (1)
1-4: LGTM! Well-organized imports with clear separation of concerns.
The imports are properly structured, separating React core imports, internal primitives, and component-specific context.
src/core/primitives/Floater/index.tsx (1)
1-17: Verify HoverCard implementation dependencies
The changes look good and provide all necessary utilities for implementing the HoverCard component. Let's verify that these utilities are being used correctly in the HoverCard implementation.
✅ Verification successful
Let's verify the actual implementation details to ensure the utilities are being used correctly.
Floater utilities are correctly integrated in HoverCard implementation
The verification confirms that the Floater utilities are properly integrated in the HoverCard component:
- Essential positioning utilities (
useFloating) with correct middleware setup (arrow, offset, flip) - Interaction handlers (
useHover,useInteractions) with appropriate configuration - All imported utilities from the Floater module are being utilized effectively
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of Floater utilities in HoverCard implementation
# Check for HoverCard component files
echo "Checking HoverCard implementation..."
fd -e tsx -e ts "HoverCard" src/
# Verify usage of floating-ui utilities
echo "Verifying utility usage..."
rg -l "useFloating|useHover|FloatingArrow" $(fd -e tsx -e ts "HoverCard" src/)
Length of output: 1091
Script:
#!/bin/bash
# Check the implementation of HoverCardRoot where floating utilities are used
echo "Checking HoverCardRoot implementation..."
rg -A 10 "useFloating|useHover|FloatingArrow" src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
# Check if other Floater utilities are used in the HoverCard components
echo -e "\nChecking usage of other Floater utilities..."
rg -l "FloatingPortal|FloatingOverlay|FloatingFocusManager|useDismiss|flip|shift|hide|offset" src/components/ui/HoverCard/
Length of output: 1136
src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx (3)
1-6: 🛠️ Refactor suggestion
Add proper TypeScript types and styling utilities
The imports section needs enhancement for better type safety and styling capabilities.
Apply these changes:
-import React, { useContext } from 'react';
+import React, { useContext, forwardRef } from 'react';
+import { type ComponentPropsWithoutRef } from 'react';
+import { cn } from '@/lib/utils';
import HoverCardContext from '../contexts/HoverCardContext';
-import Primitive from '~/core/primitives/Primitive';
+import { Primitive } from '@/components/ui/primitive';7-9:
Add proper TypeScript types and ref forwarding
The component needs proper type definitions and ref forwarding for better type safety and reusability.
Apply these changes:
-const HoverCardTrigger = ({ children, ...props }) => {
+interface HoverCardTriggerProps extends ComponentPropsWithoutRef<typeof Primitive.span> {
+ asChild?: boolean;
+}
+
+const HoverCardTrigger = forwardRef<HTMLSpanElement, HoverCardTriggerProps>(
+ ({ children, ...props }, forwardedRef) => {
const { floatingRefs, closeWithDelay, openWithDelay } = useContext(HoverCardContext);
+ if (!floatingRefs || !closeWithDelay || !openWithDelay) {
+ throw new Error('HoverCardTrigger must be used within HoverCardProvider');
+ }Likely invalid or redundant comment.
7-17: Verify integration with parent HoverCard component
Let's ensure this component is properly integrated with the parent HoverCard component.
✅ Verification successful
HoverCard integration is properly implemented
The component is correctly integrated with its parent HoverCard component:
- The context is properly defined in
HoverCardContext.tsxwith required state and handlers HoverCardRootprovides the context with necessary values includingopenWithDelayandcloseWithDelayhandlersHoverCardTriggercorrectly consumes the context and applies the event handlers- Event handling is consistent between trigger and content components, both using the same delay handlers
- Component composition follows the expected pattern as shown in
HoverCard.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check HoverCard component integration and event handling
# Check for proper component composition
echo "Checking HoverCard component composition..."
rg -A 10 "export .* HoverCard" --type typescript --type tsx
# Check for proper event handler implementation
echo "\nChecking event handler implementation..."
ast-grep --pattern 'const HoverCard = $_ => {
$$$
onMouseEnter={$_}
$$$
onMouseLeave={$_}
$$$
}'
# Check for proper context provider implementation
echo "\nChecking context provider implementation..."
rg -A 5 "HoverCardProvider" --type typescript --type tsx
Length of output: 590
Script:
#!/bin/bash
# Let's try a different approach to verify the integration
# Find all HoverCard-related files
echo "Finding HoverCard-related files..."
fd -e tsx -e ts "HoverCard"
# Check for context implementation
echo -e "\nChecking context implementation..."
rg "HoverCardContext" -A 5
# Check for event handlers in parent component
echo -e "\nChecking event handlers in parent..."
rg "openWithDelay|closeWithDelay" -A 5
# Look for usage patterns
echo -e "\nChecking usage patterns..."
rg "<HoverCard" -A 5
Length of output: 12286
src/components/ui/HoverCard/fragments/HoverCardContent.tsx (1)
8-10: Verify floating UI setup and configuration
The component uses floating UI features (floatingRefs, floatingStyles, getFloatingProps). Let's verify the complete floating UI setup.
Also applies to: 31-33
✅ Verification successful
Floating UI integration is properly configured and implemented
The floating UI setup is complete and well-structured:
@floating-ui/reactis properly installed as a dependency- Core floating UI features are correctly exported through the
Floaterprimitive HoverCardRootimplements proper floating UI configuration with:- Placement and strategy settings
- Middleware setup (arrow, offset, flip)
- Proper interaction hooks (useRole, useDismiss, useHover)
HoverCardContentcorrectly consumes the floating UI props and applies them
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for floating UI configuration and dependencies
echo "Checking floating UI setup..."
# Check package.json for floating UI dependencies
cat package.json | jq '.dependencies | keys[] | select(contains("@floating-ui"))'
# Look for floating UI configuration
rg -l "@floating-ui" src/
Length of output: 376
Script:
#!/bin/bash
# Let's check the actual implementation and usage of floating UI in the component
# First, let's see the full HoverCardContent implementation
cat src/components/ui/HoverCard/fragments/HoverCardContent.tsx
# Then check for any floating UI setup in parent components
cat src/components/ui/HoverCard/HoverCard.tsx 2>/dev/null || true
# Also check the Floater primitive since it seems to be a core component for floating UI
cat src/core/primitives/Floater/index.tsx
Length of output: 2450
Script:
#!/bin/bash
# Let's check the HoverCardRoot implementation where the floating UI setup should be
cat src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
# Also check the context implementation
cat src/components/ui/HoverCard/contexts/HoverCardContext.tsx
Length of output: 4111
src/components/ui/HoverCard/stories/HoverCard.stories.js (1)
1-2: Verify the root path alias and consider adding type definitions
The ~ alias path needs to be properly configured in the build system. Also, consider adding PropTypes or TypeScript definitions for better type safety.
✅ Verification successful
Root path alias is correctly configured and SandboxEditor component exists
The ~ alias is properly configured in all build configurations (tsconfig.json, rollup.config.cjs, and jest.config.cjs) and maps to src/*. The SandboxEditor component exists at the correct location (src/components/tools/SandboxEditor/SandboxEditor.tsx) and is being used appropriately in the HoverCard stories. No changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the path alias is configured in common build config files
fd -g '*config*' -t f -x grep -l "~" {}
Length of output: 97
Script:
#!/bin/bash
# Check the configuration of the ~ alias in the config files
echo "=== tsconfig.json paths ==="
grep -A 5 "paths" tsconfig.json || true
echo -e "\n=== rollup.config.cjs alias ==="
grep -A 5 "alias" rollup.config.cjs || true
echo -e "\n=== jest.config.cjs moduleNameMapper ==="
grep -A 5 "moduleNameMapper" jest.config.cjs || true
echo -e "\n=== Verify SandboxEditor component exists ==="
fd -g "SandboxEditor.{js,jsx,ts,tsx}" components/tools/SandboxEditor
Length of output: 1471
Script:
#!/bin/bash
# Search for SandboxEditor component in the src directory
echo "=== Looking for SandboxEditor component ==="
fd -g "SandboxEditor.{js,jsx,ts,tsx}" src
echo -e "\n=== Check if the component is actually used in HoverCard stories ==="
rg "SandboxEditor" "src/components/ui/HoverCard/stories/HoverCard.stories.js" -A 5
Length of output: 1110
styles/themes/default.scss (2)
13-13: LGTM! Import statement follows conventions.
The hover-card component import is correctly placed alphabetically among other component imports, maintaining consistency with the existing file structure.
Line range hint 71-91: Verify potential style conflicts with existing tooltip styles.
The file contains existing tooltip and popper styles. Since hover cards typically use similar floating elements and positioning mechanisms, please verify that there are no style conflicts or unintended interactions between these components.
Let's check for potential style conflicts:
✅ Verification successful
No style conflicts detected between hover-card and tooltip components
Based on the analysis of the codebase:
- The hover-card component has its own isolated styles in
styles/themes/components/hover-card.scsswith scoped class names (.rad-ui-hover-card) - Tooltip styles in
styles/themes/default.scssare separately scoped with.rad-ui-tooltipclasses - Both components use their own namespaced classes and don't share overlapping style declarations
- The z-index values are properly managed, with no competing z-index declarations between these components
The components appear to be well-isolated through proper CSS scoping, and there are no apparent style conflicts or unintended interactions between the hover-card and tooltip/popper styles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential style conflicts between hover-card and tooltip/popper styles
# Search for hover card related styles and classes
echo "Checking hover card styles:"
rg -l "hover-card|hovercard" --type scss
# Search for tooltip/popper related styles that might conflict
echo -e "\nChecking for potential conflicts with tooltip/popper styles:"
rg -A 3 "position:|z-index:|transform:|transition:" --type scss $(fd -e scss -e css)
Length of output: 1438
Script:
#!/bin/bash
# Let's try a different approach using just ripgrep without file type specification
echo "Checking hover card styles and related positioning:"
rg -A 5 "hover-?card"
echo -e "\nChecking tooltip and popper styles:"
rg -A 5 "tooltip|popper"
echo -e "\nChecking positioning and z-index declarations:"
rg -A 3 "position:|z-index:"
Length of output: 10982
src/core/primitives/Primitive/index.tsx (2)
3-5: Definition of supported elements is appropriate
The SUPPORTED_HTML_ELEMENTS constant and the SupportedElement type are correctly defined, ensuring only specified HTML elements are used.
8-11: PrimitiveProps interface extends HTMLAttributes correctly
The PrimitiveProps interface appropriately extends React.HTMLAttributes<HTMLElement> and includes optional asChild and children properties.
| const HoverCardContext = createContext<HoverCardContextType>({ | ||
| isOpen: false, | ||
| handleOpenChange: () => {} | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check protection and create a custom hook
The context could benefit from null check protection and a custom hook for better type safety and usage.
+const DEFAULT_CONTEXT: HoverCardContextType = {
+ isOpen: false,
+ handleOpenChange: () => {},
+};
+
-const HoverCardContext = createContext<HoverCardContextType>({
- isOpen: false,
- handleOpenChange: () => {}
-});
+const HoverCardContext = createContext<HoverCardContextType>(DEFAULT_CONTEXT);
+
+if (process.env.NODE_ENV !== 'production') {
+ HoverCardContext.displayName = 'HoverCardContext';
+}
+
+export function useHoverCard() {
+ const context = React.useContext(HoverCardContext);
+ if (context === undefined) {
+ throw new Error('useHoverCard must be used within a HoverCardProvider');
+ }
+ return context;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const HoverCardContext = createContext<HoverCardContextType>({ | |
| isOpen: false, | |
| handleOpenChange: () => {} | |
| }); | |
| const DEFAULT_CONTEXT: HoverCardContextType = { | |
| isOpen: false, | |
| handleOpenChange: () => {}, | |
| }; | |
| const HoverCardContext = createContext<HoverCardContextType>(DEFAULT_CONTEXT); | |
| if (process.env.NODE_ENV !== 'production') { | |
| HoverCardContext.displayName = 'HoverCardContext'; | |
| } | |
| export function useHoverCard() { | |
| const context = React.useContext(HoverCardContext); | |
| if (context === undefined) { | |
| throw new Error('useHoverCard must be used within a HoverCardProvider'); | |
| } | |
| return context; | |
| } |
| const HoverCardArrow = ({ ...props }) => { | ||
| const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext); | ||
|
|
||
| return <Floater.Arrow className={`${rootClass}-arrow`} {...props} context={floatingContext} ref={arrowRef} />; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript types for better type safety and documentation.
The component could benefit from explicit type definitions for better maintainability and developer experience.
Consider applying these changes:
-const HoverCardArrow = ({ ...props }) => {
+interface HoverCardArrowProps extends React.ComponentPropsWithoutRef<typeof Floater.Arrow> {}
+
+const HoverCardArrow = ({ className, ...props }: HoverCardArrowProps) => {
const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext);
- return <Floater.Arrow className={`${rootClass}-arrow`} {...props} context={floatingContext} ref={arrowRef} />;
+ return (
+ <Floater.Arrow
+ className={className ? `${rootClass}-arrow ${className}` : `${rootClass}-arrow`}
+ {...props}
+ context={floatingContext}
+ ref={arrowRef}
+ />
+ );
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const HoverCardArrow = ({ ...props }) => { | |
| const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext); | |
| return <Floater.Arrow className={`${rootClass}-arrow`} {...props} context={floatingContext} ref={arrowRef} />; | |
| }; | |
| interface HoverCardArrowProps extends React.ComponentPropsWithoutRef<typeof Floater.Arrow> {} | |
| const HoverCardArrow = ({ className, ...props }: HoverCardArrowProps) => { | |
| const { floatingContext, arrowRef, rootClass } = useContext(HoverCardContext); | |
| return ( | |
| <Floater.Arrow | |
| className={className ? `${rootClass}-arrow ${className}` : `${rootClass}-arrow`} | |
| {...props} | |
| context={floatingContext} | |
| ref={arrowRef} | |
| /> | |
| ); | |
| }; |
| useFloating, | ||
| Arrow: FloatingArrow, | ||
| arrow, | ||
| useRole, | ||
| useInteractions, | ||
| useDismiss, | ||
| useHover, | ||
| flip, | ||
| shift, | ||
| hide, | ||
| offset |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve organization and add documentation
The Floater object could benefit from:
- Type-safe organization of exports
- Documentation for each utility
- Logical grouping of related functionality
Consider restructuring like this:
interface FloaterType {
// Components
Portal: typeof FloatingPortal;
Overlay: typeof FloatingOverlay;
FocusManager: typeof FloatingFocusManager;
Arrow: typeof FloatingArrow;
// Hooks
useFloating: typeof useFloating;
useRole: typeof useRole;
useInteractions: typeof useInteractions;
useDismiss: typeof useDismiss;
useHover: typeof useHover;
// Middleware
arrow: typeof arrow;
flip: typeof flip;
shift: typeof shift;
hide: typeof hide;
offset: typeof offset;
}
/**
* Floater provides utilities for building floating UI components like tooltips,
* popovers, and hover cards.
*/
const Floater: FloaterType = {
// Components
Portal: FloatingPortal,
Overlay: FloatingOverlay,
FocusManager: FloatingFocusManager,
Arrow: FloatingArrow,
// Hooks
useFloating,
useRole,
useInteractions,
useDismiss,
useHover,
// Middleware
arrow,
flip,
shift,
hide,
offset,
};| return <> | ||
| <Primitive.span | ||
| onClick={() => {}} | ||
| onMouseEnter={openWithDelay} onMouseLeave={closeWithDelay} | ||
| ref={floatingRefs.setReference} | ||
| asChild>{children}</Primitive.span> | ||
| </>; |
There was a problem hiding this comment.
Improve accessibility and remove unnecessary wrapper
The component needs proper ARIA attributes and the empty onClick handler should be removed.
Apply these changes:
- return <>
- <Primitive.span
- onClick={() => {}}
- onMouseEnter={openWithDelay} onMouseLeave={closeWithDelay}
- ref={floatingRefs.setReference}
- asChild>{children}</Primitive.span>
- </>;
+ return (
+ <Primitive.span
+ {...props}
+ ref={(node) => {
+ floatingRefs.setReference(node);
+ if (typeof forwardedRef === 'function') forwardedRef(node);
+ else if (forwardedRef) forwardedRef.current = node;
+ }}
+ onMouseEnter={openWithDelay}
+ onMouseLeave={closeWithDelay}
+ aria-haspopup="dialog"
+ role="button"
+ tabIndex={0}
+ >
+ {children}
+ </Primitive.span>
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return <> | |
| <Primitive.span | |
| onClick={() => {}} | |
| onMouseEnter={openWithDelay} onMouseLeave={closeWithDelay} | |
| ref={floatingRefs.setReference} | |
| asChild>{children}</Primitive.span> | |
| </>; | |
| return ( | |
| <Primitive.span | |
| {...props} | |
| ref={(node) => { | |
| floatingRefs.setReference(node); | |
| if (typeof forwardedRef === 'function') forwardedRef(node); | |
| else if (forwardedRef) forwardedRef.current = node; | |
| }} | |
| onMouseEnter={openWithDelay} | |
| onMouseLeave={closeWithDelay} | |
| aria-haspopup="dialog" | |
| role="button" | |
| tabIndex={0} | |
| > | |
| {children} | |
| </Primitive.span> | |
| ); |
| if (!isOpen) return null; | ||
| return <div | ||
| onPointerEnter={openWithDelay} | ||
| onPointerLeave={closeWithDelay} | ||
| className={`${rootClass}`} {...props} | ||
| ref={floatingRefs.setFloating} | ||
| style={floatingStyles} | ||
| {...getFloatingProps()}>{children}</div>; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and className handling
- Add ARIA attributes for better accessibility
- Use a proper className handling utility
- Consider debouncing pointer events
if (!isOpen) return null;
return <div
onPointerEnter={openWithDelay}
onPointerLeave={closeWithDelay}
- className={`${rootClass}`} {...props}
+ className={clsx(rootClass, props.className)}
+ role="tooltip"
+ aria-hidden={!isOpen}
ref={floatingRefs.setFloating}
style={floatingStyles}
{...getFloatingProps()}>{children}</div>;Committable suggestion skipped: line range outside the PR's diff.
…delay to 100 and close delay to 200
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/ui/HoverCard/fragments/HoverCardRoot.tsx (2)
22-23: Consider extracting magic numbers as named constantsThe
ARROW_HEIGHTandSPACING_GAPvalues should be defined as named constants at the file level. Consider making these configurable via props if they need to be customizable.+const DEFAULT_ARROW_HEIGHT = 8; +const DEFAULT_SPACING_GAP = 2; + const HoverCardRoot = ({ children, open: controlledOpen = undefined, onOpenChange, customRootClass = '', openDelay = 100, closeDelay = 200, + arrowHeight = DEFAULT_ARROW_HEIGHT, + spacingGap = DEFAULT_SPACING_GAP, ...props }: HoverCardRootProps) => { - const ARROW_HEIGHT = 8; - const SPACING_GAP = 2;
25-37: Make placement configurable via propsThe placement is currently hardcoded to 'bottom'. Consider making it configurable via props to support different positioning scenarios.
+type Placement = 'top' | 'bottom' | 'left' | 'right'; + const HoverCardRoot = ({ children, open: controlledOpen = undefined, onOpenChange, customRootClass = '', openDelay = 100, closeDelay = 200, + placement = 'bottom' as Placement, ...props }: HoverCardRootProps) => { // ... const { refs: floatingRefs, floatingStyles, context: floatingContext } = Floater.useFloating({ - placement: 'bottom', + placement, // ... });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/ui/HoverCard/fragments/HoverCardRoot.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx(1 hunks)src/components/ui/HoverCard/stories/HoverCard.stories.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx
- src/components/ui/HoverCard/stories/HoverCard.stories.js
🔇 Additional comments (3)
src/components/ui/HoverCard/fragments/HoverCardRoot.tsx (3)
9-17: Props type definition needs refinement
The props type definition still needs to be updated to properly extend HTML attributes and match the actual usage in the component.
68-71: Remove console.log statements
The console.log statements should be removed from production code.
Also applies to: 74-77
3-3: Verify HoverCardContext type safety
Let's verify that the HoverCardContext is properly typed to match the values being provided.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
src/components/ui/HoverCard/contexts/HoverCardContext.tsx (2)
17-18: Avoid usinganytype forfloatingContextandarrowRefUsing
anydefeats the purpose of TypeScript's type safety. Please specify the appropriate types forfloatingContextandarrowRefto enhance type safety and code maintainability.Suggested changes:
-import { createContext } from 'react'; +import { createContext, MutableRefObject } from 'react'; +import type { FloatingContext } from '@floating-ui/react'; type HoverCardContextType = { // ... - floatingContext: any; - arrowRef: any; + floatingContext: FloatingContext; + arrowRef: MutableRefObject<HTMLDivElement | null>; };
3-20: Organize context properties for better readabilityConsider grouping related properties together or ordering them alphabetically to improve the readability and maintainability of the
HoverCardContextTypedefinition.src/components/ui/HoverCard/fragments/HoverCardContent.tsx (1)
12-20: Add null check protection when consuming contextTo prevent potential runtime errors if the context is undefined, add a null check when consuming
HoverCardContext.Apply this diff to safely consume the context:
const HoverCardContent = ({ children, ...props }: HoverCardContentProps) => { - const { + const context = useContext(HoverCardContext); + if (!context) { + throw new Error('HoverCardContent must be used within a HoverCardProvider'); + } + const { isOpen, floatingRefs, floatingStyles, getFloatingProps, rootClass, closeWithDelay, closeWithoutDelay, openWithDelay - } = useContext(HoverCardContext); + } = context;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/components/ui/HoverCard/HoverCard.tsx(1 hunks)src/components/ui/HoverCard/contexts/HoverCardContext.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardContent.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardPortal.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardRoot.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx
🔇 Additional comments (7)
src/components/ui/HoverCard/contexts/HoverCardContext.tsx (1)
22-22: 🛠️ Refactor suggestion
Provide a default context value and create a custom hook for safer context consumption
Initializing HoverCardContext with an empty object cast to HoverCardContextType can lead to runtime errors if any of the context properties are accessed before they are set. It's recommended to provide a default context value with default implementations, and create a custom hook to safely consume the context.
Apply this diff to implement the changes:
+const DEFAULT_CONTEXT: HoverCardContextType = {
+ isOpen: false,
+ handleOpenChange: () => {},
+ floatingRefs: {
+ setReference: () => {},
+ setFloating: () => {},
+ },
+ getReferenceProps: () => ({}),
+ getFloatingProps: () => ({}),
+ floatingStyles: {},
+ rootClass: '',
+ closeWithDelay: () => {},
+ closeWithoutDelay: () => {},
+ openWithDelay: () => {},
+ floatingContext: null,
+ arrowRef: null,
+};
-const HoverCardContext = createContext<HoverCardContextType>({} as HoverCardContextType);
+const HoverCardContext = createContext<HoverCardContextType>(DEFAULT_CONTEXT);
+export function useHoverCardContext() {
+ const context = useContext(HoverCardContext);
+ if (!context) {
+ throw new Error('useHoverCardContext must be used within a HoverCardProvider');
+ }
+ return context;
+}src/components/ui/HoverCard/fragments/HoverCardPortal.tsx (1)
1-1:
Implement portal functionality using createPortal
The HoverCardPortal component currently renders a div without utilizing React's portal mechanism. To ensure that the hover card content is rendered outside of the parent DOM hierarchy (avoiding clipping and z-index issues), import createPortal from 'react-dom' and use it to render the content into document.body.
Apply this diff to implement the portal:
-import React from 'react';
+import React from 'react';
+import { createPortal } from 'react-dom';
type HoverCardPortalProps = {
children: React.ReactNode,
- props: React.HTMLAttributes<HTMLElement>
+} & React.HTMLAttributes<HTMLElement>;
const HoverCardPortal = ({ children, ...props }: HoverCardPortalProps) => {
- return <div {...props}>{children}</div>;
+ return createPortal(
+ <div {...props}>{children}</div>,
+ document.body
+ );
};Also applies to: 8-10
src/components/ui/HoverCard/fragments/HoverCardContent.tsx (2)
35-35: Enhance className handling and add ARIA attributes for accessibility
Use a utility like clsx to handle conditional class names and avoid issues when props.className is undefined. Additionally, add ARIA attributes to improve accessibility.
Apply this diff to improve className handling and accessibility:
+import clsx from 'clsx';
onPointerEnter={openWithDelay}
onPointerLeave={closeWithDelay}
- className={`${rootClass}`} {...props}
+ className={clsx(rootClass, props.className)}
+ role="tooltip"
+ aria-hidden={!isOpen}
ref={floatingRefs.setFloating}
style={floatingStyles}
{...getFloatingProps()}23-29: 🛠️ Refactor suggestion
Throttle the scroll event handler to improve performance
Attaching an unthrottled scroll listener to the window can lead to performance issues due to the high frequency of scroll events. Throttle the handleScroll function to improve performance.
Apply this diff to throttle the scroll handler:
+import throttle from 'lodash.throttle';
useEffect(() => {
- const handleScroll = () => closeWithoutDelay();
+ const handleScroll = throttle(() => closeWithoutDelay(), 100);
window.addEventListener('scroll', handleScroll);
return () => {
window.removeEventListener('scroll', handleScroll);
+ handleScroll.cancel();
};
}, [closeWithoutDelay]);src/components/ui/HoverCard/fragments/HoverCardRoot.tsx (3)
57-59: 🛠️ Refactor suggestion
Use openDelay and closeDelay props in useHover hook
The useHover hook currently uses a fixed delay. Update it to use the openDelay and closeDelay props to match the desired timings for the hover behavior.
Apply this diff to use delay props:
const hover = Floater.useHover(floatingContext, {
- delay: 100
+ delay: { open: openDelay, close: closeDelay }
});75-93:
Prevent potential memory leaks by clearing timeouts
The setTimeout functions in openWithDelay and closeWithDelay can lead to memory leaks if not properly managed. Store the timeouts and clear them when necessary.
Apply this diff to manage timeouts:
+import { useEffect, useRef } from 'react';
const HoverCardRoot = ({ ... }: HoverCardRootProps) => {
+ const timeoutRef = useRef<NodeJS.Timeout | null>(null);
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ };
+ }, []);
const openWithDelay = () => {
markMouseIsEntering();
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
handleOpenChange(true);
}, openDelay);
};
const closeWithDelay = () => {
markMouseIsExiting();
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
setMouseIsExiting(prevState => {
if (prevState) {
handleOpenChange(false);
}
return prevState;
});
}, closeDelay);
};9-17: 🛠️ Refactor suggestion
Refactor HoverCardRootProps to properly extend HTML attributes
The HoverCardRootProps type currently defines a props property, which can cause confusion when spreading ...props. Instead, extend React.HTMLAttributes<HTMLDivElement> to include all standard HTML attributes.
Apply this diff to adjust the prop types:
-type HoverCardRootProps = {
- children: React.ReactNode,
- open: boolean | undefined,
- onOpenChange: (open: boolean) => void,
- customRootClass: string,
- openDelay: number,
- closeDelay: number,
- props?: React.HTMLAttributes<HTMLElement>
-}
+type HoverCardRootProps = React.HTMLAttributes<HTMLDivElement> & {
+ children: React.ReactNode;
+ open?: boolean;
+ onOpenChange?: (open: boolean) => void;
+ customRootClass?: string;
+ openDelay?: number;
+ closeDelay?: number;
+};
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
src/components/ui/AlertDialog/fragments/AlertDialogPortal.tsx (1)
11-11: Consider adding development-mode warning for missing root elementWhile falling back to document.body is reasonable, silently doing so could mask configuration issues. Consider adding a development-mode warning when the specified rootClass element is not found.
const rootElement = document.getElementsByClassName(rootClass)[0] as HTMLElement | null; +if (process.env.NODE_ENV === 'development' && !rootElement) { + console.warn(`[AlertDialogPortal] No element found with class "${rootClass}", falling back to document.body`); +} return ( <Floater.Portal root={rootElement || document.body}Also applies to: 15-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/components/ui/AlertDialog/fragments/AlertDialogPortal.tsx(1 hunks)src/components/ui/HoverCard/HoverCard.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardPortal.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardRoot.tsx(1 hunks)src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ui/HoverCard/fragments/HoverCardTrigger.tsx
- src/components/ui/HoverCard/fragments/HoverCardRoot.tsx
🔇 Additional comments (2)
src/components/ui/HoverCard/fragments/HoverCardPortal.tsx (1)
5-9:
Fix props type definition
The current type definition has issues:
- The
propsproperty will cause type conflicts - The
rootElementtype could be more specific
type HoverCardPortalProps = {
children: React.ReactNode,
- rootElement: HTMLElement | React.MutableRefObject<HTMLElement | null> | undefined,
- props: React.HTMLAttributes<HTMLElement>
-}
+ rootElement?: HTMLElement | React.MutableRefObject<HTMLElement | null>,
+} & React.HTMLAttributes<HTMLElement>Likely invalid or redundant comment.
src/components/ui/HoverCard/HoverCard.tsx (1)
22-22:
Add required accessibility attributes
The HoverCard needs proper accessibility attributes for better screen reader support.
- content = undefined,
+ content,
// ...
<HoverCardPortal>
- <HoverCardContent>
+ <HoverCardContent
+ role="tooltip"
+ aria-describedby="hover-card-content"
+ >
{content}
<HoverCardArrow />
</HoverCardContent>Also applies to: 40-45
Demo -
https://github.com/user-attachments/assets/d9459483-8b07-49ff-8d11-5ae26740f299
Summary by CodeRabbit
Summary by CodeRabbit
HoverCardcomponent with subcomponents:HoverCardRoot,HoverCardTrigger,HoverCardPortal,HoverCardContent, andHoverCardArrowfor enhanced hover interactions.HoverCardcomponent.HoverCardcomponent enhance its visual appearance.These changes improve user interaction by providing a dynamic hover effect with customizable content and a polished design.