Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

fix(action): Support slotting icon after initialization #913

Merged
merged 4 commits into from
Apr 2, 2020

Conversation

driskull
Copy link
Member

@driskull driskull commented Apr 2, 2020

Related Issue: None

Summary

fix(action): Support slotting icon after initialization

@driskull driskull added bug Something isn't working 1 - assigned labels Apr 2, 2020
@driskull driskull added this to the KOO (King of Ooo) milestone Apr 2, 2020
@driskull driskull requested a review from jcfranco April 2, 2020 23:10
@driskull driskull requested a review from a team as a code owner April 2, 2020 23:10
@driskull driskull self-assigned this Apr 2, 2020
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

👍

@driskull driskull merged commit faddbc3 into master Apr 2, 2020
@driskull driskull deleted the dris0000/fix-action-children-after-init branch April 2, 2020 23:44

buttonEl: HTMLButtonElement;

observer = new MutationObserver(() => this.setHasChildren());
Copy link
Member

Choose a reason for hiding this comment

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

Would using forceUpdate after a mutation change help to simplify?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that but wasn't sure if it was as efficient as using a state variable. Do you know?

Copy link
Member

Choose a reason for hiding this comment

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

Haven't really played with it, but I'd think it's similar. Every time there's a mutation change you update state to rerender. Using forceUpdate seems like it would skip the setting state part. Worth testing.

@driskull driskull mentioned this pull request Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 - assigned bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants