Add onClick function in the AccordionData #516
Conversation
WalkthroughThe changes in this pull request enhance the functionality of the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/core/Accordion/types.ts (1)
9-9: LGTM! Consider adding documentation and refining the type.The addition of the optional
onClickproperty to theAccordionDatatype is correct and aligns with the PR objectives. It allows for custom click behavior on individual accordion items without breaking existing usage.Consider the following improvements:
- Add JSDoc comments to explain the purpose and usage of this new property.
- Consider refining the type to include an event parameter for more flexibility:
onClick?: (event: React.MouseEvent<HTMLElement>) => void;This would allow users to access event information if needed, while still maintaining the option to ignore it.
src/core/Accordion/Accordion.stories.tsx (2)
46-49: LGTM! Consider using a more descriptive alert message.The new
dataWithCustomClickableconstant correctly extends the existingdataarray with a customonClickhandler, aligning with the PR objectives. This implementation allows for demonstrating custom click functionality in the Storybook.Consider using a more descriptive alert message to better illustrate the custom click functionality:
const dataWithCustomClickable = data.map((datum, index) => ({ ...datum, - onClick: () => alert("Custom onClick"), + onClick: () => alert(`Custom onClick for ${datum.name}`), }));This change would make it clearer which item was clicked in the Storybook demo.
213-226: LGTM! Consider enhancing the documentation.The new
CustomClickableButtonstory effectively demonstrates the custom clickable functionality for accordion items. The implementation aligns well with the PR objectives.Consider enhancing the documentation to provide more context and guidance:
parameters: { docs: { description: { story: - "Supplying an `onClick` function to a section adds a clickable function. This can be useful for custom actions.", + "Supplying an `onClick` function to a section adds a custom clickable function. This can be useful for implementing specific actions or behaviors when an accordion item is clicked. In this example, clicking an item triggers an alert, but in real-world scenarios, this could be used for navigation, data fetching, or any other custom interaction.", }, }, },This expanded description provides more context on the feature's potential uses, which could be helpful for developers using the component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/core/Accordion.tsx (1 hunks)
- src/core/Accordion/Accordion.stories.tsx (2 hunks)
- src/core/Accordion/types.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/core/Accordion.tsx (1)
206-209: LGTM! Enhanced accordion item click handling.The changes successfully implement the custom
onClickhandler for accordion items as per the PR objectives. The implementation:
- Preserves the original toggle functionality.
- Adds the ability to execute a custom click handler if provided.
- Maintains backwards compatibility by using optional chaining.
This enhancement provides greater flexibility for developers using the
Accordioncomponent, allowing for additional interactive functionality within accordion items.src/core/Accordion/Accordion.stories.tsx (1)
Line range hint
1-226: Summary: Excellent implementation of custom click handlers for accordion items.The changes in this file effectively implement and demonstrate the new custom click functionality for accordion items. The additions are well-integrated with the existing code structure and follow the established patterns for Storybook stories.
Key points:
- The new
dataWithCustomClickableconstant provides a clear example of how to add custom click handlers to accordion items.- The
CustomClickableButtonstory effectively showcases the new functionality.- The changes align perfectly with the PR objectives of enhancing the
Accordioncomponent with customonClickhandlers.These modifications provide developers with more flexibility in implementing interactive behaviors within the
Accordioncomponent. The Storybook demonstration will be valuable for understanding and utilizing this new feature.
aralovelace
force-pushed
the
Accordion_add_onClick
branch
from
86e2861
to
40d3bee
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
src/core/icons/icon-display-chat-mono.svgis excluded by!**/*.svgsrc/core/icons/icon-display-data-broadcast-mono.svgis excluded by!**/*.svgsrc/core/icons/icon-display-equalisers-mono.svgis excluded by!**/*.svgsrc/core/icons/icon-display-push-notifications-mono.svgis excluded by!**/*.svgsrc/core/icons/icon-display-support-chat-mono.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
- package.json (1 hunks)
- src/core/Accordion.tsx (1 hunks)
- src/core/Accordion/Accordion.stories.tsx (2 hunks)
- src/core/Accordion/types.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/core/Accordion.tsx
- src/core/Accordion/Accordion.stories.tsx
- src/core/Accordion/types.ts
🧰 Additional context used
🔇 Additional comments (1)
package.json (1)
Line range hint
1-145: Verify completeness of changes.The PR objectives and AI summary mention changes to the Accordion component, including updates to its functionality and Storybook stories. However, these changes are not reflected in the package.json file.
Please ensure that all necessary changes have been included in this pull request. You may want to run the following command to check for any uncommitted changes:
#!/bin/bash # Check for any uncommitted changes git status --porcelainIf this command returns any output, it indicates that there are uncommitted changes that might need to be included in this PR.
aralovelace
force-pushed
the
Accordion_add_onClick
branch
2 times, most recently
from
4a225db
to
aa51511
Compare
aralovelace
force-pushed
the
Accordion_add_onClick
branch
from
aa51511
to
91a554b
Compare
|
@jamiehenson thanks Jamie, I passed an index but I do not need to pass anything so I leave it for now. I will ask @kennethkalmer for re-review thanks |
There was a problem hiding this comment.
I'm grateful you took on @jamiehenson's feedback around passing something, like the index, to the onClick handler. It can be too easy on the consuming side to get confused and long for a way to disambiguate between callbacks.
Jira Ticket Link / Motivation
WEB-4036
Summary of changes
This pull request introduces a new feature to the
Accordioncomponent, allowing customonClickhandlers for each item. Additionally, it includes changing svg files so we can change its colorNew feature in Accordion component:
src/core/Accordion.tsx: Updated theonClickhandler to call a customonClickfunction if provided.src/core/Accordion/types.ts: Added an optionalonClickproperty to theAccordionDatatype.src/core/Accordion/Accordion.stories.tsx: Added a new story to demonstrate the customonClickfunctionality. [1] [2]Version update:
package.json: Bumped version from14.7.4to14.7.5.How do you manually test this?
Test The Accordion component in the application
Reviewer Tasks (optional)
?path=/story/js-components-accordion--with-custom-on-clickMerge/Deploy Checklist
Frontend Checklist
Summary by CodeRabbit
New Features
Documentation
onClickfunctionality in the Accordion component stories.Chores
package.jsonto reflect the latest development changes.