Skip to content

Fix feature loading on soft navigation between React-based pages#8881

Draft
SunsetTechuila wants to merge 8 commits intorefined-github:mainfrom
SunsetTechuila:events
Draft

Fix feature loading on soft navigation between React-based pages#8881
SunsetTechuila wants to merge 8 commits intorefined-github:mainfrom
SunsetTechuila:events

Conversation

@SunsetTechuila
Copy link
Copy Markdown
Member

@SunsetTechuila SunsetTechuila commented Jan 20, 2026

@SunsetTechuila SunsetTechuila marked this pull request as ready for review January 20, 2026 15:29
@fregante
Copy link
Copy Markdown
Member

 one-event 4.4.0 published 🎉

@fregante fregante added the bug label Jan 22, 2026
}
});
} while (await oneEvent(document, 'turbo:render'));
} while (await oneEvent(document, ['turbo:render', 'soft-nav:react-done']));
Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Jan 22, 2026

Choose a reason for hiding this comment

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

unlike soft-nav:render, soft-nav:react-done is fired when user navigates the session history too

@SunsetTechuila SunsetTechuila changed the title Load features when navigating between React-based pages Fix features loading on soft navigation between React-based pages Jan 27, 2026
@SunsetTechuila SunsetTechuila changed the title Fix features loading on soft navigation between React-based pages Fix feature loading on soft navigation between React-based pages Jan 27, 2026
document.addEventListener('turbo:before-fetch-request', unloadAll); // Clicks
document.addEventListener('turbo:visit', unloadAll); // Back/forward button
document.addEventListener('soft-nav:progress-bar:start', unloadAll); // Soft navigation
document.addEventListener('soft-nav:start', unloadAll); // Soft navigation
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a bit too early, but other events are fired too late

Comment on lines +131 to +132
document.addEventListener('soft-nav:start', unloadAll); // Soft navigation
addEventListener('popstate', unloadAll); // History navigation
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you reckon that we can proceed without these two early events or do they cause too much trouble/noise in the console?

What does "firing too early" mean? Does the page break visually?

Copy link
Copy Markdown
Member Author

@SunsetTechuila SunsetTechuila Jan 28, 2026

Choose a reason for hiding this comment

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

What does "firing too early" mean? Does the page break visually?

Here is a demonstration:

msedge_prwsHOQhvG

signal.addEventListener('abort', () => {
descriptionBanner.remove();
});

signal.addEventListener('abort', () => {
disabledBanner.remove();
});

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now I think why not unload on soft-nav:render? Seems to work well

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you reckon that we can proceed without these two early events

We can't. Neither the turbo:before-fetch-request nor the turbo:visit events are fired in the test cases I provided in the PR description

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Now I think why not unload on soft-nav:render?

I've let this idea marinate in my head, and I think it's too risky

@SunsetTechuila SunsetTechuila marked this pull request as draft February 3, 2026 22:47
SunsetTechuila added a commit that referenced this pull request Feb 3, 2026
To see the issue or test the fix:

1. Rebase this branch on #8881
2. Go to #4008
3. Hover the first mention
4. Click #3980
@fregante
Copy link
Copy Markdown
Member

I'm planning to release 26.3.14, this PR won't make the cut so don't force yourself to finish it

@SunsetTechuila
Copy link
Copy Markdown
Member Author

SunsetTechuila commented Mar 13, 2026

Current status of this PR:

Next steps:

  • Replace at least the soft-nav:start listener with soft-nav:render. We might also want to remove turbo:before-fetch-request
  • After that, we need to do extensive testing to make sure nothing breaks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants