Skip to content

Collapse, ExpansionPanel and Accordion components#101

Merged
pixelbandito merged 5 commits intocision:masterfrom
hpieroni:expansion-panel-component
Sep 10, 2019
Merged

Collapse, ExpansionPanel and Accordion components#101
pixelbandito merged 5 commits intocision:masterfrom
hpieroni:expansion-panel-component

Conversation

@hpieroni
Copy link
Copy Markdown
Contributor

@hpieroni hpieroni commented Sep 3, 2019

This pr includes the following components:

  • Chevron icon
  • Collapse: wraps Transition component from react-transition-group and animates height.
  • ExpansionPanel: includes a header (with an optional icon) and a body that is collapsed/expanded via Collapse component. It could be a controlled or uncontrolled component.
  • Accordion: receives multiple ExpansionPanel components as children and allows only one of them to be expanded at the same time.

Copy link
Copy Markdown

@rheckart rheckart left a comment

Choose a reason for hiding this comment

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

Looks good to me! Will be interested in any thoughts that the primary stakeholders have.

Comment thread src/components/Accordion/index.js Outdated
Comment thread src/components/Accordion/index.js
Comment thread src/components/Accordion/story.js
Comment thread src/components/Accordion/test.js
Comment thread src/components/Collapse/index.js
Comment thread src/components/ExpansionPanel/index.js
@pixelbandito
Copy link
Copy Markdown
Contributor

👏 👏 👏
Nice job with tests, and very thorough! I left a few comments, but this looks like an awesome contribution to RoverUI!

Copy link
Copy Markdown
Contributor

@mdespuits mdespuits left a comment

Choose a reason for hiding this comment

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

Loving all of the tests! A few suggestions for sharing children prop types but otherwise looks 👍

Comment thread src/components/Accordion/index.js Outdated
Comment thread src/components/ExpansionPanel/Body/index.js Outdated
Comment thread src/components/ExpansionPanel/Body/index.js Outdated
Comment thread src/components/ExpansionPanel/Header/index.js Outdated
Copy link
Copy Markdown
Contributor

@pixelbandito pixelbandito left a comment

Choose a reason for hiding this comment

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

Left a small, optional note.

Comment thread src/components/ExpansionPanel/Body/index.js Outdated
@hpieroni hpieroni force-pushed the expansion-panel-component branch from d5cff94 to 8e0d8db Compare September 5, 2019 00:34
@hpieroni hpieroni requested a review from mdespuits September 6, 2019 13:24
@pixelbandito pixelbandito merged commit e6977c1 into cision:master Sep 10, 2019
Comment thread example/src/App.js
</section>
<section>
<h1>Accordion</h1>
<Accordion>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You didn't import Accordion. :(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@hpieroni hpieroni deleted the expansion-panel-component branch September 10, 2019 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants