Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wizard modal glitch #198

Merged
merged 1 commit into from Apr 17, 2023
Merged

fix: wizard modal glitch #198

merged 1 commit into from Apr 17, 2023

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Apr 17, 2023

Motivation

The wizard modal opening is glitchy on desktop which really annoys me.

The problem

The wizard modal is glitchy because we noticed issue last year which we solved by wrapping the transition component within a if that became true only once the modal intro animation was done. This is the major root cause of the issue. Because the transition is not displayed, the slot it contains is not displayed and per extension the content is not displayed, therefore the height is not correct.

In addition to be glitchy, this is also an issue for the browser as if such guard is removed without further code, it can lead the browser to become irresponsive as the component transition ends in an infinite loop (⚠️ 🚨).

After opening and closing the modal a zillion times this afternoon I came to the conclusion that the root cause of the issue is the onDestroy side effect of the WizardModal that is sometimes not called when the modal is used in NNS-dapp.

This problem looks like the Svelte issue 👉 sveltejs/svelte#5268

To overcome the issue, I observed the nnsClose event of the modal and force not rendering its content when this happens which has for effect to destroy the transition ✅.

See screenshots for demos.

Changes

  • in WizardModal, on nnsClose, set a flag to false to destroy / not render its content instead of waiting for introEnd
  • remove the scale and replace with a fade effect

Screenshots

Current glitch:

git-glitch-1

Current glitch slowed down:

gif-glitch-2

New animation effect without glitch:

git-new-animation

Issue and infinite loop problem if only introEnd is removed:

gif-infinite-loop

No issue if nnsClose is used to force deletion of content (no infinite loop happening after a while):

gif-loop-fix-3

@peterpeterparker peterpeterparker added the bug Something isn't working label Apr 17, 2023
Copy link
Contributor

@mstrasinskis mstrasinskis left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@peterpeterparker peterpeterparker merged commit 653ac53 into main Apr 17, 2023
7 checks passed
@peterpeterparker peterpeterparker deleted the fix/wizard-modal-glitch branch April 17, 2023 14:54
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this pull request Apr 17, 2023
# Motivation

Fix wizard modal opening glitch by integrating gix-cmp PR
dfinity/gix-components#198

# ⚠️ PRs

The related PR is a tricky PR!

- [x] dfinity/gix-components#198

# Changes

- bump gix-cmp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants