-
Notifications
You must be signed in to change notification settings - Fork 113
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
Allow expand "AccordionItem" programmatically #13
Allow expand "AccordionItem" programmatically #13
Conversation
👍 related: #12 @vincentaudebert do you have time to review this in the near future? |
I will try to fit this in this week or the next one. Exciting! |
@epotockiy would it be possible that you provide some examples or ideally that you test your changes so we get 100% coverage? |
@vincentaudebert I'll try to do it. |
Any progress @epotockiy ? |
@vincentaudebert I added some tests for new functionality, could you review this? |
It's looking really good to me @epotockiy ! Thanks a lot for your amazing work! |
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.
👍 I added some notes for minor changes.
<td>activeItems</td> | ||
<td>Array</td> | ||
<td>[]</td> | ||
<td>Indexes (or custom keys) to pre expand items. Can be changed dynamically. Doesn't have the priority against `AccordionItem - expanded` on first render.</td> |
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.
Would be nice to have some other documentation mentioning this customKey
prop.
src/Accordion/accordion.js
Outdated
@@ -28,17 +34,38 @@ class Accordion extends Component { | |||
this.renderItems = this.renderItems.bind(this); | |||
} | |||
|
|||
componentWillReceiveProps(nextProps) { | |||
if (!isArraysEqual(nextProps.activeItems, this.state.activeItems)) { |
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.
Rename this to isArraysEqualShallow
to make sure there is no confusion about what is going on here?
@@ -0,0 +1,19 @@ | |||
// eslint-disable-next-line |
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.
This override doesn't seem necessary – I imagine it's to circumvent the rule "no solo const
export in a file". Could this be moved as an export default
in a separate file instead?
…tically-expand * upstream/master: Update README.md labelledby for all type of bodies more coverage for item title set linting rule in config adds documentation about accessibility fixes springload#18 0.5.0 updating changelog bumping to node 8 Modifies publish documentation Changed perms of githooks' deploy script to be executable. (springload#15) Update docs to reflect changes. Add 'hideBodyClassName' prop to AccordionItemTitle component. Add 'hideBodyClassName' prop to AccordionItem component
Merged. Thanks again @epotockiy will be released into |
Add
activeItems
prop realization for programmatically controlling of expanded Accordion items #12