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

Set transitions as |local to solve populating destroy #2865

Merged
merged 4 commits into from
Jul 12, 2023

Conversation

peterpeterparker
Copy link
Member

@peterpeterparker peterpeterparker commented Jul 12, 2023

Motivation

Svelte transitions are executed by default in components regardless if their parent components destroy those or not. This behavior is known as the |global transitions.

This leads NNS dapp to various issues in which components are not destroyed and therefore, there context remains active.

Until today we solved this by applying a workaround in which we destroyed the all layout in case of navigation. While it solved the issue, this had for side effect to be a bit unpleasant for the eyes as the all page layout disappeared on navigation.

By setting all the animation to |local we can solve the issue and make the navigation more fluid.

References

PRs

Previous issues

Changes

  • update gix-cmp to set |local to any Svelte transitions
  • set |local to the Svelte animations implemented in NNS dapp
  • remove detail page layout navigating guard

Notes

In the login page we will continue to observe $navigating because this condition was not only used to solve the issue but, also to present a spinner while the application is loading.

@peterpeterparker peterpeterparker added the feature request New feature or request label Jul 12, 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.

Thx!

@peterpeterparker peterpeterparker added this pull request to the merge queue Jul 12, 2023
Merged via the queue into main with commit 0c1000b Jul 12, 2023
37 checks passed
@peterpeterparker peterpeterparker deleted the feat/local-transitions-destroy branch July 12, 2023 08:02
github-merge-queue bot pushed a commit that referenced this pull request Jul 13, 2023
# Motivation

Reverting layout based fix for missing onDestroy after disbursing sns
neuron ([more info](#2861))

# PRs

Revert PR #2865

# Changes

- apply the workaround from the wallet page to all detail pages.
- set local to global animation flag

# Tests

Tested manually

# Todos

- [ ] Add entry to changelog (if necessary).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants