fix: Improve fast-click behavior#12966
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Improves sidebar “fast-click” navigation behavior to avoid single-frame active-state flashes by deriving active matching from the pending navigation target before React Router’s location updates.
Changes:
- Replaces per-link
preActivestate with a shared pending-navigation location used for active matching across all sidebarNavLinks. - Tracks whether the link was already active at
mousedownto preventonActiveClickfrom firing on fast-click navigations. - Removes forced remounting via
keyand wrapsNavLinkwithmobx-reactobserverto react to pending-navigation state.
Comments suppressed due to low confidence (1)
app/components/Sidebar/components/NavLink.tsx:98
toLocationis recomputed on every render (and for stringtoit creates a freshLocationviacreateLocation). With the newhandleMouseDowndependency list includingtoLocation, this can cause the callback to be recreated on unrelated re-renders and adds extracreateLocationchurn across many sidebar links.
const toLocation = normalizeToLocation(
resolveToLocation(to, currentLocation),
currentLocation
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
+38
| // The target of a fast-click navigation, shared between all NavLinks so that | ||
| // only links matching it can render as active before the location changes. | ||
| const pendingLocation = observable.box<Location | null>(null, { deep: false }); | ||
|
|
||
| const setPendingLocation = action((location: Location | null) => { | ||
| pendingLocation.set(location); | ||
| }); |
This file contains hidden or 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
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.
Subtle details in fast click behavior that prevent single frame flashes
NavLinktracked its optimistic “pre-active” state locallykey={isActive ? "active" : "inactive"}remount hack and replaced it with an explicit guard ononActiveClick