fix: broken onDestroy on navigation #2880
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- reverting because the switching to |local breaks introend event
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 17, 2023
# Motivation When a Sns neuron is successfully disbursed, the modal in which the action is triggered is closed and the user is automatically redirected to the list of neurons. As a result, we noticed that the page and components `onDestroy` were not called. We fixed the issue by adding a guard at the top of the layout. Today I debugged the issue again and figured out that the root cause of the particular issue was the fact that the navigation was executed before the dispatcher that closes the modal was triggered. I couldn't replicate the issue with a sample repo but, by inverting both order, can solve the issue. # Notes This PR does not remove the guard. Only fix the particular issue in case we would remove it in the future. # PRs Follow-up various PR but, last is #2880 # Changes - invert `goto` and `dispatch("nnsClose")`
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jul 25, 2023
# Motivation Implementing the workaround for `onDestroy` at the top level of the `+layout` of the detail pages (PR #2880) had for side effect to make the navigation callbacks of its pages/layouts to not being called. As those are not called, the dapp is not able to determine which was the `referrer` of these pages and per extension, to effectivelly use the referrer in back action. Long story short, on mainnet, navigation from the "Launchpad" to a "Proposal" and clicking back lead the users to "Proposals" page instead of going back to "Launchpad". # Changes - refactor workaround in a dedicated component `LayoutNavGuard` - use component within the layouts - remove top layout as unused # Notes I noticed a type in the CHANGELOG and since I was already there, I also improved the communication of the fix #2950 which is not related to this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Reverting layout based fix for missing onDestroy after disbursing sns neuron (more info)
PRs
Revert PR #2865
Changes
Tests
Tested manually