fix(modal): OpenCloseComponent emits when setting --calcite-duration-factor to 0
#5326
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
…nsitionDurationProp === 0s
…-transition-duration-0
github-actions
bot
added
the
bug
github-actions
bot
added
the
Stale
…-transition-duration-0
…-transition-duration-0
…rop by parsing an aggregate properties string from .getPropertyValue(transition) query
jcfranco
reviewed
Oct 5, 2022
…-transition-duration-0
…dundant typing, and give more info on how/when the function should be used
…-transition-duration-0
…gle function call onToggleOpenCloseComponent(this)
… a single way for handling both the duration > 0 and duration === 0 cases
…-transition-duration-0
…once invoked. Remove the openModal() when component loads as this won't be needed in this pattern
…-transition-duration-0
Elijbet
removed
the
Stale
…-transition-duration-0
…-transition-duration-0
driskull
reviewed
Nov 22, 2022
driskull
reviewed
Nov 22, 2022
| @@ -134,6 +130,7 @@ export class Modal | |||
| setUpLoadableComponent(this); | |||
| // when modal initially renders, if active was set we need to open as watcher doesn't fire | |||
| if (this.open) { | |||
| onToggleOpenCloseComponent(this); | |||
There was a problem hiding this comment.
Calling this here will emit these events on load of the component. Is there some history on why we are emitting on load here? Usually for components, we don't emit events initially, we start emitting after load on user interaction.
If this is intentional, the test will need to be updated to expect the event to be called initially.
There was a problem hiding this comment.
The discussion on this is moved to #4398.
|
This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions. |
github-actions
bot
added
the
Stale
alisonailea
reviewed
Dec 1, 2022
alisonailea
requested changes
Dec 1, 2022
…-transition-duration-0
9 tasks
Elijbet
removed
the
Stale
…-transition-duration-0
…it the test for emitting on inital render to one with duration and one with none
…-transition-duration-0
Elijbet
added
the
pr ready for visual snapshots
alisonailea
approved these changes
Dec 6, 2022
Elijbet
deleted the
elijbet/5206-OpenCloseComponents-emit-with-transition-duration-0
branch
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.
Related Issue: #5206
All (before)open/close events are now emitted on modal regardless of transition duration.
Add the
onToggleUnanimatedComponentfunction to theopenCloseComponent Interfaceto query props on thetransitionEl. Use that to get thetransition-durationon theopenTransitionProp, which in the case of modal it’s theopacity. This will allow us to emit the open/close and beforeOpen/beforeClose events in cases whereopacitytransition duration is set to 0.Because calling computed style forces a layout, we use
readTask()instead which schedules a DOM-read task. The provided callback will be executed at the best moment to perform DOM reads without causing layout thrashing as per Stencil docs.Updated to rework
openCloseComponent Interfaceto call one function instead of 3 separate calls in every component by removing connect/disconnect and event listener mapping logic and passing argumentonceto.addEventListenerontransitionElinstead.