Skip to content

fix(menu): do not error if disabled or swipeGesture is changed mid-animation - #28268

Merged
liamdebeasi merged 13 commits into
mainfrom
FW-4120
Oct 9, 2023
Merged

fix(menu): do not error if disabled or swipeGesture is changed mid-animation#28268
liamdebeasi merged 13 commits into
mainfrom
FW-4120

Conversation

@liamdebeasi

@liamdebeasi liamdebeasi commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

Issue number: resolves #20092, resolves #19676, resolves #19000


What is the current behavior?

Menu is currently throwing errors because it expects no animations to be running when any state changes happen (such as changing disabled or swipeGesture).

For example, if you set swipeGesture="false" mid-gesture then the menu will error. Alternatively, if you set disabled="true" mid-open animation then the menu will error also. This is undesirable because it can cause visual flickering and other undesirable behaviors as noted in the linked threads.

What is the new behavior?

  • Any in-progress animation is cancelled if the state updates such that the animation is no longer relevant (i.e. disabled is set to true while the menu is opening)
  • Removed relevant assertions
  • Added tests

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build: 7.4.3-dev.11696264821.1755dd6a

@github-actions github-actions Bot added the package: core @ionic/core package label Oct 2, 2023
@liamdebeasi liamdebeasi changed the title Fw 4120 fix(menu): do not error if disabled or swipeGesture is changed mid-animation Oct 2, 2023
@liamdebeasi
liamdebeasi marked this pull request as ready for review October 2, 2023 17:01
@liamdebeasi
liamdebeasi requested a review from a team October 2, 2023 17:01
@liamdebeasi

Copy link
Copy Markdown
Contributor Author

I'm holding off on merging until next week to give developers on the linked threads time to test and provide feedback.

Base automatically changed from animation-abort to main October 9, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants