Fix Wrong Page Underlined as Current Page in Left Nav Bar#1391
Open
esu-skoopin wants to merge 14 commits intoprocessing:mainfrom
Open
Fix Wrong Page Underlined as Current Page in Left Nav Bar#1391esu-skoopin wants to merge 14 commits intoprocessing:mainfrom
esu-skoopin wants to merge 14 commits intoprocessing:mainfrom
Conversation
…er than use global "jumpToState" variable
…yout>, rather than use global "jumpToState" variable
…site Accidentally set "heading" prop for "jumpToState" prop for <BaseLayout> for both in previous commits; fixed that in this commit
…s" subsection of the site
…ection of the site
…ople" section of the site
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.
Resolves #1304
Perceived Cause Behind Bug
The main issue, as far as we know, is the combination of a build configuration that allows for the site to be built concurrently two pages at a time and the use of the global variable,
jumpToState.This is the relevant build setting:
We can take a simulated look at what's happening to better understand the problem:
jumpToStatejumpToStatefor page AjumpToStatefor page BjumpToStatefor page AjumpToStatefor page BBecause the site is being built concurrently two pages at a time, it's possible for
jumpToStateto be overwritten by page B before page A can read fromjumpToState.This can result in the visual bug, as seen below.
Current page incorrectly underlined as "Map"
Current page correctly underlined as "Map"
*The assumption is that these pages were built concurrently at build time and "Map" overwrote
jumpToStatebefore "Linear Interpolation" could read from itOur Solution
For our solution, we didn't want to simply change the build config for the site, as we believe that this config setting was most likely put in place for a good reason, so we instead decided to replace the use of the global
jumpToStatevariable with a "jumpToState" prop for the <BaseLayout> and <Nav> components.jumpToStatevariable↓
Global
jumpToStatevariable potentially gets overwritten by another page↓
<Nav jumpToState={global
jumpToStatevariable}>↓
<Nav jumpToState={page}>
Screenshots of our solution working as expected locally:
Feedback welcome!