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

Transitions block page navigation #6686

Closed
benmccann opened this issue Aug 27, 2021 · 6 comments
Closed

Transitions block page navigation #6686

benmccann opened this issue Aug 27, 2021 · 6 comments

Comments

@benmccann
Copy link
Member

Describe the problem

Transitions can block page navigation if |local is not set (sveltejs/kit#628)

Describe the proposed solution

Consider whether to make |local the default for transitions

Alternatives considered

I couldn't think of another solution

Importance

would make my life easier

@benmccann benmccann added this to the 4.x milestone Aug 27, 2021
@antony
Copy link
Member

antony commented Aug 27, 2021

Transitions and navigation have always been a pain. Nobody wants to see two pages at once, piled on top of eachother, whilst waiting for a transition to finish - it's unexpected. To my mind:

  • local should be the default, absolutely.
  • navigation should wait for a transition to complete - this is only acceptable if |local is the default

If we made the above change, page transitions in SvelteKit would be as seamless and easy as transitions in Svelte are generally.

@7antra
Copy link

7antra commented Aug 27, 2021

About that, we had a little conversation in discord with Brady Fractal and navigation is not only the use case which transitions are painful.

I'm using svelte:self to create a recursive dropdown for a menu. And I use the slide transition which is great. Only I can't use |local by default since it's the parent component that will impact on the child in its "expanded" state. However, I don't want the transition to happen when changing the page (navigation).

Today there is only |local; but there are many cases where a more nuanced control of whether this transition should be triggered or not would be useful.

Would this granularity be possible one day, according to Svelte's design ?

@stalkerg
Copy link
Contributor

I totally agree with @7antra based on my experience the "navigation" it's not equal to page changes.
If we introduce |local by default we should add |global option as well. :)
Also, looks like this is important mostly for SvelteKit users, but we still have many SPA without Kit.

@benmccann
Copy link
Member Author

|local will be the default in v4

@205g0
Copy link

205g0 commented Jul 4, 2023

Just read the docs, this and another issue about |local as default. I'd love to get an ELI5 on this change:

  1. Concrete use cases for |local
  2. Why not |global as default?

I really didn't get the point, I am happy to take local as new default but would love to understand it better since I've never had any problems with page navigation and transitions (maybe because I use always absolute positioned pages? IDK), so happy to get some better explanation.

peterpeterparker added a commit to dfinity/gix-components that referenced this issue 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 setting and is the default in Svelte v3.

This leads to issue with the SvelteKit navigation because DOM elements might not be detached and related context might remain active.

We generally notice the issue in NNS dapp after navigation when the DOM wrongly contains two `split-pane` elements.

In Svelte v4 the default behavior was changed to `|local`. Setting that can already be used in v3 when explicitely set. This setting has for effect to not execute the transition and destroy the components if parents destroy those. i.e. to populate the destroy.

# References

- documentation: https://svelte.dev/docs/v4-migration-guide#transitions-are-local-by-default
- svelte issue: sveltejs/svelte#6686

# Changes

- scope all transitions of Svelte to `|local`
github-merge-queue bot pushed a commit to dfinity/nns-dapp that referenced this issue 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

- documentation:
https://svelte.dev/docs/v4-migration-guide#transitions-are-local-by-default
- svelte issue: sveltejs/svelte#6686

# PRs

- [x] dfinity/gix-components#246

# Previous issues

- #1741
- #2861

# 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants