-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(MenuButton): 14063 Allow to pass popover classname as a prop. Fix passing className prop to MenuList, MenuPopover, DropdownPopover #100
Conversation
WalkthroughThe changes involve modifications to several components within the UI kit, primarily focusing on how class names are constructed and passed to components. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropdownPopover
participant MenuList
participant MenuPopover
participant Popover
User->>DropdownPopover: Interacts with dropdown
DropdownPopover->>MenuList: Renders menu list
MenuList->>MenuPopover: Passes custom classes
MenuPopover->>Popover: Renders popover with styles
Popover->>User: Displays popover
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (4)
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 (
|
♻️ PR Preview 2008af1 has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (4)
- packages/ui-kit/src/Dropdown/components/DropdownPopover/index.tsx (1 hunks)
- packages/ui-kit/src/MenuButton/components/MenuList/index.tsx (1 hunks)
- packages/ui-kit/src/MenuButton/components/MenuPopover/index.tsx (1 hunks)
- packages/ui-kit/src/Popover/index.tsx (1 hunks)
Additional comments not posted (5)
packages/ui-kit/src/MenuButton/components/MenuList/index.tsx (2)
20-26
: LGTM!The changes to the
MenuList
component are approved:
- The destructuring of the
className
andclasses
properties aligns with the updatedMenuListProps
interface.- The construction of the
dynamicClasses
variable using thecn
function is a common and correct pattern for conditionally applying classes in React components.
29-29
: LGTM!The change to pass the
classes?.popover
value as theclassName
prop to theMenuPopover
component is approved. This allows for custom styling of the popover through theclasses
prop of theMenuList
component, enhancing flexibility.packages/ui-kit/src/MenuButton/components/MenuPopover/index.tsx (1)
24-30
: LGTM!The changes to the
dynamicClasses
variable construction improve code readability by expanding the object passed tocn
into a more explicit block format. This change does not alter the functionality of theMenuPopover
component and may facilitate easier future modifications or additions to the class names being applied.The changes are consistent with the AI-generated summary.
packages/ui-kit/src/Dropdown/components/DropdownPopover/index.tsx (1)
87-87
: LGTM!The changes to the
cn
function call enhance the flexibility of class name composition and potentially improve readability and maintainability without altering the overall functionality of theDropdownPopover
component.packages/ui-kit/src/Popover/index.tsx (1)
48-53
: LGTM!The changes to the
dynamicClasses
variable declaration improve code readability by clearly delineating the object being passed to thecn
function. The logical functionality remains unaffected.
We need to pass z-index to MenuPopover, otherwise it gets hidden by panels beneath it. This PR adds
classes
prop to MenuList, which allows passingpopover
classname.It also fixes passing className prop to MenuList, MenuPopover, DropdownPopover (it was not working correctly)
Summary by CodeRabbit
New Features
MenuList
component with the addition of aclasses
property, allowing users to define specific styles for theMenuPopover
.Bug Fixes
DropdownPopover
,MenuPopover
, andPopoverImpl
components without altering their functionality.