fix: left nav duplicate active selection and jumping scroll

[jnm2377]

Closes #386
Closes #345

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/carbon-design-system/gatsby-theme-carbon/nkdzoo89a
✅ Preview: https://gatsby-theme-carbon-git-fork-jnm2377-left-nav-jump.carbon-design-system.now.sh

[vpicone]

Lookin good! do you mind doing a quick video of how we can test this/see it in action?

[jnm2377]

@sstrubberg I just pushed what I'm testing with. The duplicate accordion nav items. Is that still showing the issue for you?

[sstrubberg]

Huzzah! not sure what was different, but both the deploy preview and my local environment are singing.

However, that linting error is still there.

[sstrubberg]

Should we just give it an // eslint-disable-line react-hooks/exhaustive-deps?

[vpicone]

@sstrubberg @jnm2377 those eslint react hooks rules from the react team are super well thought out, I'm down for disabling some rules for old code but we it looks like they've caught something that's a little off here.

[vpicone]

Since the left nav is the only thing that cares about the scroll position, I'm not sure it makes sense to lift it all the way up to the app level context. Gatsby let's you pass state to a link, maybe we pass that into our side nav items and use it via location.state to set the position after the page loads. https://www.gatsbyjs.org/docs/gatsby-link/#pass-state-as-props-to-the-linked-page

[vpicone]

I think it's looking great. Only issue now is when there's two menu's open and one closes after navigation, the new scroll position is off. Verified with @aagonzales that the open state of the menus should persist through navigation. That way the mouse is still over the target they interacted with.

[jnm2377]

@vpicone @sstrubberg this is ready for re-review.