add MutationObserver to accordion to trigger setChildContentHeight when children change #947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…en children change
cchaos
approved these changes
Jun 26, 2018
chandlerprall
suggested changes
Jun 26, 2018
| @@ -44,6 +44,15 @@ export class EuiAccordion extends Component { | |||
|
|
|||
| componentDidMount() { | |||
| this.setChildContentHeight(); | |||
|
|
|||
| requestAnimationFrame(() => { | |||
There was a problem hiding this comment.
Did you test this without requestAnimationFrame? I don't think it needs the delay.
… add comment to change log
chandlerprall
suggested changes
Jun 26, 2018
| @@ -44,6 +44,13 @@ export class EuiAccordion extends Component { | |||
|
|
|||
| componentDidMount() { | |||
| this.setChildContentHeight(); | |||
|
|
|||
| this.observer = new MutationObserver(this.setChildContentHeight); | |||
| this.observer.observe(this.childContent, { childList: true, subtree: true }); | |||
There was a problem hiding this comment.
Last thing, sorry for not catching this before.
If this.childContent changes to a new ref before componentWillUnmount is called the new content isn't tracked. This observer creation should be moved to where this.childContent is set (currently an inline arrow function, that will need to be promoted to a method on the component class). The ref setting function should check if the incoming ref is null or not; if this.observer exists, disconnect; then if ref is not null, create new observer.
The setRef method will be called with null on unmount, so there is no need to keep componentWIllUnmount
chandlerprall
approved these changes
Jun 26, 2018
|
jenkins, test this |
cchaos
pushed a commit
to cchaos/eui
that referenced
this pull request
Jun 28, 2018
commit 2eea143 Author: cchaos <caroline.horn@elastic.co> Date: Thu Jun 28 12:56:56 2018 -0400 Get rid of terrible IE highlight when select is focused commit 66647f0 Author: cchaos <caroline.horn@elastic.co> Date: Thu Jun 28 12:07:54 2018 -0400 Revert commit of filter_button.scss commit c2e8a73 Author: cchaos <caroline.horn@elastic.co> Date: Thu Jun 28 12:00:54 2018 -0400 Simplifying form control styles - Made bottom colored border from linear-gradient - Allowing parameters to determine if just the borders should render - Allowing parameters to determine if all state styles should be added - Fixing variable naming schemes commit c5f0ab5 Author: Stacey Gammon <gammon@elastic.co> Date: Thu Jun 28 10:02:16 2018 -0400 add inspect to typedef file (elastic#952) * add inpsect to typedef file * use the right pr number commit c64f055 Author: Nathan Reese <reese.nathan@gmail.com> Date: Wed Jun 27 08:52:41 2018 -0600 add MutationObserver to accordion to trigger setChildContentHeight when children change (elastic#947) * add MutationObserver to accordion to trigger setChildContentHeight when children change * remove requestAnimationFrame around MutationObserver registration and add comment to change log * set up observer in ref creation function * mock MutationObserver
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
EuiAccordion does not update height when children internal state changes. I updated the AccordionGrow example to demonstrate. When the counter is modified inside the sub-component than EuiAccordion's componentDidUpdate never gets called because none of the props passed to children changed.
This PR adds a MutationObserver to update height any time the children DOM tree changes.