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.

Sign up for GitHub

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

Jump to bottom

Add support for accordion component #291

Merged
merged 15 commits into from
Oct 25, 2021
Merged

Add support for accordion component #291

merged 15 commits into from
Oct 25, 2021

Conversation

adirhaz1z
Copy link
Contributor

Screen Shot 2021-10-21 at 14 11 41

Screen Shot 2021-10-21 at 14 11 36

@hadasfa hadasfa changed the title Created Accordion in StoryBook WIP:Created Accordion in StoryBook Oct 21, 2021
@hadasfa hadasfa self-requested a review October 21, 2021 15:08
hadasfa
hadasfa reviewed Oct 21, 2021
View reviewed changes
src/components/Accordion/Accordion/Accordion.jsx
const children = useMemo(() => {
const allChildren = React.Children.toArray(originalChildren);
return allChildren.filter(child => {
if (child.type.isAccordionChild) return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not instance of check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing done in Menu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to isSubMenu? That is a different case because in that case, we use the same component for the menu and the submenu - here, we are using different components

hadasfa
hadasfa suggested changes Oct 24, 2021
View reviewed changes
Copy link
Contributor

@hadasfa hadasfa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greate pr! I wrote some notes :)

hadasfa
hadasfa reviewed Oct 25, 2021
View reviewed changes
package.json Outdated Show resolved Hide resolved
hadasfa
hadasfa reviewed Oct 25, 2021
View reviewed changes
src/components/Accordion/Accordion/Accordion.jsx
children: null,
defaultIndex: []
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful component! And using the Accordion Item as children instead of an object is awesome! :)

hadasfa
hadasfa reviewed Oct 25, 2021
View reviewed changes
src/components/ExpandCollapse/ExpandCollapse.jsx

const toogleExpand = () => {
setIsOpen(!isOpen);
};
const renderHeader = useCallback(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay!

@hadasfa hadasfa changed the title WIP:Created Accordion in StoryBook Created Accordion in StoryBook Oct 25, 2021
@hadasfa hadasfa changed the title Created Accordion in StoryBook Add support for accordion component Oct 25, 2021
@hadasfa hadasfa merged commit 4b65911 into mondaycom:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hadasfa hadasfa Awaiting requested review from hadasfa

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adirhaz1z @hadasfa