Skip to content
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

Extend menu component props #144

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Extend menu component props #144

merged 1 commit into from
Oct 30, 2024

Conversation

skrobek
Copy link
Collaborator

@skrobek skrobek commented Oct 30, 2024

PR Type

enhancement, tests


Description

  • Added isExpanded property to MenuItem interface and updated Menu component to utilize this property for controlling the default open state of details elements.
  • Updated the Menu story to demonstrate the use of the isExpanded property.
  • Incremented the package version to 0.11.4 to reflect the new enhancements.

Changes walkthrough 📝

Relevant files
Enhancement
Menu.tsx
Add `isExpanded` property to Menu component                           

src/components/ui/menu/Menu.tsx

  • Added isExpanded property to menu items.
  • Updated details element to use isExpanded for default open state.
  • +10/-2   
    types.ts
    Extend `MenuItem` interface with `isExpanded`                       

    src/components/ui/menu/types.ts

    • Added isExpanded property to MenuItem interface.
    +1/-0     
    Tests
    Menu.stories.tsx
    Update Menu story to include `isExpanded` example               

    src/stories/Menu.stories.tsx

    • Added isExpanded property to a menu item in the story example.
    +1/-0     
    Configuration changes
    package.json
    Bump package version to 0.11.4                                                     

    package.json

    • Updated package version from 0.11.3 to 0.11.4.
    +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Default Prop Handling
    Ensure that the default values for props like className and onClick are handled correctly to avoid potential runtime errors or unexpected behaviors.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Prevent rendering empty dropdown components

    Use conditional rendering to ensure that the details element is only included when
    there are children to display, preventing empty dropdowns.

    src/components/ui/menu/Menu.tsx [56-60]

    -<details open={isExpanded}>
    -  <summary className={itemClassNames} onClick={onClick}>
    -    {label}
    -  </summary>
    -  <ul className='before:w-0'>{children.map(renderMenItem)}</ul>
    +{children && (
    +  <details open={isExpanded}>
    +    <summary className={itemClassNames} onClick={onClick}>
    +      {label}
    +    </summary>
    +    <ul className='before:w-0'>{children.map(renderMenItem)}</ul>
    +  </details>
    +)}
    Suggestion importance[1-10]: 9

    Why: This suggestion effectively prevents the rendering of empty dropdowns, enhancing the user interface by ensuring that dropdowns only appear when there are children to display.

    9
    Possible bug
    Ensure onClick is callable to prevent runtime errors

    Add a check to ensure that onClick is a function before using it as an event handler
    to prevent runtime errors if undefined or a non-function value is passed.

    src/components/ui/menu/Menu.tsx [51]

    -<a className={itemClassNames} onClick={onClick}>
    +<a className={itemClassNames} onClick={onClick && (() => onClick())}>
    Suggestion importance[1-10]: 8

    Why: Adding a check to ensure onClick is a function prevents potential runtime errors, which is crucial for maintaining the stability of the application.

    8
    Possible issue
    Prevent unintentional overriding of existing class names

    Ensure that the default value for className in the destructuring of item does not
    unintentionally override existing class names passed to the component. Consider
    appending the default class name instead of replacing it.

    src/components/ui/menu/Menu.tsx [12-19]

     const {
       icon,
       badge,
       children,
       active = false,
    -  className = '',
    +  className = `default-class ${item.className}`,
       onClick = undefined,
       isExpanded = false
     } = item;
    Suggestion importance[1-10]: 7

    Why: This suggestion addresses a potential issue where the default class name could override existing class names, improving the robustness of the component styling.

    7

    @skrobek skrobek merged commit bb5d582 into main Oct 30, 2024
    2 checks passed
    @skrobek skrobek deleted the extend-menu-component branch December 17, 2024 09:05
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant