fix: ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly #9958

jcfranco · 2024-08-03T08:06:13Z

Related Issue: #9641, #9315

Summary

This addresses some issues that prevented beforeOpen/open and beforeClose/close events from emitting as expected.

Notable changes:

Removes redundant/unused transitions in favor of ones used for open/close transitions
Updates input-date-picker to set open via a prop instead of a attribute to avoid the transition starting before onToggleOpenCloseComponent was called. We could refactor this if popover and similar components use a different, internal, attribute to control transitions.
Updates tests that would now have a transition when opening/closing
Remove unnecessary OpenCloseComponent implementation from input-time-picker as it can delegate to the internal popover.
Updates block to use on margin transition to determine when it is open or closed. I will create a refactor issue to refactor the open/close transition, focusing on the content, and will update the implementation to use this instead.
Refactored notice's styles to properly transition when opened/closed.
Added missing openClose tests to sheet and tooltip E2E test suites.
Added option to openClose test helper for tests that are expected to use the fallback due to the normal transition start/end events not being able to fire properly.
Improved openClose test helper to consider timing between beforeOpen/Close and open/close events.
Fixed whenTransitionOrAnimationDone issue that prevented the correct duration to be associated with the open/close transition.

…ltiple transitioned props

… not fired properly

packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx

driskull · 2024-08-14T21:50:18Z

packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx

@@ -978,9 +970,6 @@ export class InputTimePicker
  async componentWillLoad(): Promise<void> {
    setUpLoadableComponent(this);
    await Promise.all([setUpMessages(this), this.loadDateTimeLocaleData()]);
-    if (this.open) {
-      onToggleOpenCloseComponent(this);


With this removed, can the popover be open by default?

driskull · 2024-08-14T21:50:28Z

packages/calcite-components/src/components/input-time-picker/input-time-picker.tsx

            onCalcitePopoverClose={this.popoverCloseHandler}
            onCalcitePopoverOpen={this.popoverOpenHandler}
-            open={this.open}


With this removed, can the popover be open by default?

…te the helper doc accordingly

driskull

Looks good! ⭐

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-ui-icons: 3.31.0</summary> ## [3.31.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-ui-icons@3.30.0...@esri/calcite-ui-icons@3.31.0) (2024-08-28) ### Features * Add close-caption, transcript, flag, and flag-slash ([#10039](#10039)) ([85c7d69](85c7d69)) * Add layer-connection ([#10067](#10067)) ([46dd2ee](46dd2ee)) ### Bug Fixes * Update close-caption, and transcript ([#10077](#10077)) ([2e86904](2e86904)) </details> <details><summary>@esri/calcite-components: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.11.1...@esri/calcite-components@2.12.0) (2024-08-28) ### Features * **accordion, accordion-item:** Add component tokens ([#9861](#9861)) ([48f7f08](48f7f08)) * **action, action-bar, action-group, action-menu, action-pad:** Add component tokens ([#10058](#10058)) ([de96c24](de96c24)) * **alert:** Add 'queue' property to prioritize the ordering of alerts when opened ([#10029](#10029)) ([3aa47a4](3aa47a4)) * **combobox-item:** Add `heading` property (deprecates `textLabel`) and add `label` property ([#9987](#9987)) ([90920a0](90920a0)) * **dialog:** Add CSS variables to offset the position ([#9904](#9904)) ([660f0c5](660f0c5)) * **dialog:** Add escapeDisabled property ([#10081](#10081)) ([1ca8fba](1ca8fba)) * **dialog:** Add outsideCloseDisabled property ([#9978](#9978)) ([f1d89b9](f1d89b9)) * **dialog:** Add resizable and dragEnabled properties ([#9804](#9804)) ([e3e499e](e3e499e)) * **icon:** Add component tokens ([#10062](#10062)) ([34d5689](34d5689)) * **input-time-zone:** Display selected time zone country in region mode selection ([#9988](#9988)) ([1575729](1575729)) * **loader:** Add `determinate-value` type ([#9957](#9957)) ([7f21726](7f21726)) * **panel:** Add '--calcite-panel-content-space' style ([#10117](#10117)) ([0e3ebcb](0e3ebcb)) * **table:** Update multiple selection iconography ([#10125](#10125)) ([f2b2016](f2b2016)) * **tooltip:** Allow focusing and clicking on interactive elements within a tooltip ([#9914](#9914)) ([a3c144c](a3c144c)) ### Bug Fixes * **accordion-item:** Align focus styles across browsers ([#9944](#9944)) ([2e29b42](2e29b42)) * **alert:** Properly form a queue of alerts ([#10032](#10032)) ([b24d6ac](b24d6ac)) * **color-picker:** Prevent text selection when using color sliders/field ([#10043](#10043)) ([ab33e4d](ab33e4d)) * **combobox, input-date-picker, input-number, input-text, input-time-picker, input, radio-button-group, segmented-control, select, text-area:** Use toAriaBoolean in aria-invalid attribute to provide valid AT error messaging ([#10079](#10079)) ([78bc096](78bc096)) * **combobox:** Update placeholder-icon color ([#9956](#9956)) ([d916ca4](d916ca4)) * **dialog, flow-item:** Slotted closable panels should not close the component ([#10130](#10130)) ([ce2513d](ce2513d)) * **dialog:** Allow shell to be slotted inside ([#10118](#10118)) ([ea3a6de](ea3a6de)) * **dialog:** Fix escapeDisabled when the escape key is pressed on the panel ([#10097](#10097)) ([9a5f1f1](9a5f1f1)) * Ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly ([#9958](#9958)) ([7e2764f](7e2764f)) * **filter:** Fix setFocus method ([#10149](#10149)) ([f4a959a](f4a959a)) * **input-date-picker:** Fix selection for the "bs" and "it-CH" locales ([#9976](#9976)) ([368d9e2](368d9e2)) * **input-date-picker:** Prevent console error when using a lang ([#10162](#10162)) ([e7bf604](e7bf604)) * **input-time-zone:** Translate region names ([#9940](#9940)) ([f975adb](f975adb)) * **input, combobox, input-date-picker, input-number, input-text, input-time-picker, radio-button-group, segmented-control, select, text-area:** Provide clear field error messaging for AT ([#9880](#9880)) ([46ad7d2](46ad7d2)) * **input, input-number, input-text:** No longer set focus when a validation message is clicked ([#10008](#10008)) ([529bb5a](529bb5a)) * **label:** Prevent focusing and toggling elements slotted within a label when a text selection occurs ([#9990](#9990)) ([49d6ae1](49d6ae1)) * **list-item:** Prevent scrolling when item is focused. ([#9948](#9948)) ([eab1e06](eab1e06)) * **panel:** Define heading and description line height ([#10085](#10085)) ([5478eb9](5478eb9)) * **slider:** Prevent text selection when using the slider ([#10073](#10073)) ([04bf325](04bf325)) * **slider:** Resolve step & snap floating point precision ([#10148](#10148)) ([613bc47](613bc47)) * **stepper-item:** Update component to take full width when parent's layout is "vertical" ([#10009](#10009)) ([03a5a30](03a5a30)) * **tree,tree-item:** Replace checkbox with div and a11y attributes ([#9849](#9849)) ([b1ac951](b1ac951)) ### Performance Improvements * **input-time-zone:** Improve memory footprint by leveraging browser cache and releasing utils after their use ([#9945](#9945)) ([a3d2141](a3d2141)) ### Reverts * "build(deps): remove unused cheerio package ([#10049](#10049))" ([#10057](#10057)) ([d271ab1](d271ab1)) * Add stories ([#10131](#10131)) ([e3a6590](e3a6590)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-ui-icons bumped from 3.31.0-next.4 to 3.31.0 </details> <details><summary>@esri/calcite-components-angular: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.11.1...@esri/calcite-components-angular@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> <details><summary>@esri/calcite-components-react: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.11.1...@esri/calcite-components-react@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-ui-icons: 3.31.0</summary> ## [3.31.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-ui-icons@3.30.0...@esri/calcite-ui-icons@3.31.0) (2024-08-28) ### Features * Add close-caption, transcript, flag, and flag-slash ([#10039](#10039)) ([85c7d69](85c7d69)) * Add layer-connection ([#10067](#10067)) ([46dd2ee](46dd2ee)) ### Bug Fixes * Update close-caption, and transcript ([#10077](#10077)) ([2e86904](2e86904)) </details> <details><summary>@esri/calcite-components: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.11.1...@esri/calcite-components@2.12.0) (2024-08-28) ### Features * **accordion, accordion-item:** Add component tokens ([#9861](#9861)) ([48f7f08](48f7f08)) * **action, action-bar, action-group, action-menu, action-pad:** Add component tokens ([#10058](#10058)) ([de96c24](de96c24)) * **alert:** Add 'queue' property to prioritize the ordering of alerts when opened ([#10029](#10029)) ([3aa47a4](3aa47a4)) * **combobox-item:** Add `heading` property (deprecates `textLabel`) and add `label` property ([#9987](#9987)) ([90920a0](90920a0)) * **dialog:** Add CSS variables to offset the position ([#9904](#9904)) ([660f0c5](660f0c5)) * **dialog:** Add escapeDisabled property ([#10081](#10081)) ([1ca8fba](1ca8fba)) * **dialog:** Add outsideCloseDisabled property ([#9978](#9978)) ([f1d89b9](f1d89b9)) * **dialog:** Add resizable and dragEnabled properties ([#9804](#9804)) ([e3e499e](e3e499e)) * **icon:** Add component tokens ([#10062](#10062)) ([34d5689](34d5689)) * **input-time-zone:** Display selected time zone country in region mode selection ([#9988](#9988)) ([1575729](1575729)) * **loader:** Add `determinate-value` type ([#9957](#9957)) ([7f21726](7f21726)) * **panel:** Add '--calcite-panel-content-space' style ([#10117](#10117)) ([0e3ebcb](0e3ebcb)) * **table:** Update multiple selection iconography ([#10125](#10125)) ([f2b2016](f2b2016)) * **tooltip:** Allow focusing and clicking on interactive elements within a tooltip ([#9914](#9914)) ([a3c144c](a3c144c)) ### Bug Fixes * **accordion-item:** Align focus styles across browsers ([#9944](#9944)) ([2e29b42](2e29b42)) * **alert:** Properly form a queue of alerts ([#10032](#10032)) ([b24d6ac](b24d6ac)) * **color-picker:** Prevent text selection when using color sliders/field ([#10043](#10043)) ([ab33e4d](ab33e4d)) * **combobox, input-date-picker, input-number, input-text, input-time-picker, input, radio-button-group, segmented-control, select, text-area:** Use toAriaBoolean in aria-invalid attribute to provide valid AT error messaging ([#10079](#10079)) ([78bc096](78bc096)) * **combobox:** Update placeholder-icon color ([#9956](#9956)) ([d916ca4](d916ca4)) * **dialog, flow-item:** Slotted closable panels should not close the component ([#10130](#10130)) ([ce2513d](ce2513d)) * **dialog:** Allow shell to be slotted inside ([#10118](#10118)) ([ea3a6de](ea3a6de)) * **dialog:** Fix escapeDisabled when the escape key is pressed on the panel ([#10097](#10097)) ([9a5f1f1](9a5f1f1)) * Ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly ([#9958](#9958)) ([7e2764f](7e2764f)) * **filter:** Fix setFocus method ([#10149](#10149)) ([f4a959a](f4a959a)) * **input-date-picker:** Fix selection for the "bs" and "it-CH" locales ([#9976](#9976)) ([368d9e2](368d9e2)) * **input-date-picker:** Prevent console error when using a lang ([#10162](#10162)) ([e7bf604](e7bf604)) * **input-time-zone:** Translate region names ([#9940](#9940)) ([f975adb](f975adb)) * **input, combobox, input-date-picker, input-number, input-text, input-time-picker, radio-button-group, segmented-control, select, text-area:** Provide clear field error messaging for AT ([#9880](#9880)) ([46ad7d2](46ad7d2)) * **input, input-number, input-text:** No longer set focus when a validation message is clicked ([#10008](#10008)) ([529bb5a](529bb5a)) * **label:** Prevent focusing and toggling elements slotted within a label when a text selection occurs ([#9990](#9990)) ([49d6ae1](49d6ae1)) * **list-item:** Prevent scrolling when item is focused. ([#9948](#9948)) ([eab1e06](eab1e06)) * **panel:** Define heading and description line height ([#10085](#10085)) ([5478eb9](5478eb9)) * **slider:** Prevent text selection when using the slider ([#10073](#10073)) ([04bf325](04bf325)) * **slider:** Resolve step & snap floating point precision ([#10148](#10148)) ([613bc47](613bc47)) * **stepper-item:** Update component to take full width when parent's layout is "vertical" ([#10009](#10009)) ([03a5a30](03a5a30)) * **tree,tree-item:** Replace checkbox with div and a11y attributes ([#9849](#9849)) ([b1ac951](b1ac951)) ### Performance Improvements * **input-time-zone:** Improve memory footprint by leveraging browser cache and releasing utils after their use ([#9945](#9945)) ([a3d2141](a3d2141)) ### Reverts * "build(deps): remove unused cheerio package ([#10049](#10049))" ([#10057](#10057)) ([d271ab1](d271ab1)) * Add stories ([#10131](#10131)) ([e3a6590](e3a6590)) ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-ui-icons bumped from 3.31.0-next.4 to 3.31.0 </details> <details><summary>@esri/calcite-components-angular: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.11.1...@esri/calcite-components-angular@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-angular:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> <details><summary>@esri/calcite-components-react: 2.12.0</summary> ## [2.12.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.11.1...@esri/calcite-components-react@2.12.0) (2024-08-28) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize components versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 2.12.0-next.34 to 2.12.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> **Related Issue:** # ## Summary Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

fix: ensure open/close events emit correctly if target element has mu…

6c9fe06

…ltiple transitioned props

jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label Aug 3, 2024

github-actions bot assigned jcfranco Aug 3, 2024

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Aug 3, 2024

jcfranco added 5 commits August 5, 2024 17:01

fix copypasta

c5310b0

revisit timing test approach to avoid protocol errors when events are…

ea2b4bf

… not fired properly

fix tooltip test assertions

bdfb459

increase duration for animated cases to ensure assertions

859fb5a

drop redundant alert transition item (targets all)

ffd188a

jcfranco mentioned this pull request Aug 7, 2024

fix(input-time-zone): fix inconsistent open/close events when opened programmatically #9864

Closed

jcfranco added 6 commits August 13, 2024 15:45

fix block open/close events

cbf1743

fix notice open/close events

3f79d4a

fix input-time-picker open/close events

37f705b

adjust minimum delay used between animation and non-animation tests

9e6786a

fix block tests

6f96d21

fix focus trap regression

10b539e

jcfranco changed the title ~~fix: ensure open/close events emit correctly if target element has multiple transitioned props~~ fix: ensure beforeOpen/open and beforeClose/close events emit properly Aug 14, 2024

tidy up

4914e95

jcfranco added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed skip visual snapshots Pull requests that do not need visual regression testing. labels Aug 14, 2024

jcfranco requested review from Elijbet and driskull August 14, 2024 19:32

driskull reviewed Aug 14, 2024

View reviewed changes

jcfranco added 2 commits August 14, 2024 15:36

fix initially open input-time-picker use case

d7250c8

ensure initially open tests are in a separate describe block and upda…

8662334

…te the helper doc accordingly

jcfranco added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Aug 14, 2024

driskull approved these changes Aug 14, 2024

View reviewed changes

jcfranco merged commit 7e2764f into dev Aug 14, 2024
14 checks passed

jcfranco deleted the jcfranco/9641-fix-open-close-related-events-when-target-has-multiple-transitioned-props branch August 14, 2024 22:58

github-actions bot added this to the 2024-08-27 - Aug Release milestone Aug 14, 2024

jcfranco mentioned this pull request Aug 16, 2024

feat(alert): add 'queue' property to prioritize the ordering of alerts when opened #10029

Merged

benelan mentioned this pull request Aug 28, 2024

chore: release main #10171

Merged

benelan mentioned this pull request Aug 28, 2024

chore: release main (#10171) #10172

Merged

benelan mentioned this pull request Aug 28, 2024

chore: release main (#10171) #10173

Closed

calcite-admin mentioned this pull request Sep 30, 2024

chore: release main #10457

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly #9958

fix: ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly #9958

jcfranco commented Aug 3, 2024 •

edited

Loading

driskull Aug 14, 2024

driskull Aug 14, 2024

driskull left a comment

fix: ensure beforeOpen/open and beforeClose/close events emit properly #9958

fix: ensure beforeOpen/open and beforeClose/close events emit properly #9958

Conversation

jcfranco commented Aug 3, 2024 • edited Loading

Summary

Notable changes:

driskull Aug 14, 2024

Choose a reason for hiding this comment

driskull Aug 14, 2024

Choose a reason for hiding this comment

driskull left a comment

Choose a reason for hiding this comment

fix: ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly #9958

fix: ensure `beforeOpen`/`open` and `beforeClose`/`close` events emit properly #9958

jcfranco commented Aug 3, 2024 •

edited

Loading