remix-run / remix-website

327 stars 74 forks source link

adding active link css class for the on page side bar navigation in Remix docs #218

Closed Sarvesh-Ghildiyal closed 6 months ago

Sarvesh-Ghildiyal commented 6 months ago

Issue: While going through Remix docs, we see a on page navigation bar at the right side of the web page that helps us navigate to the content of the pages. While this is good feature one of the issue with this is, if we jump onto some content we do not actually see any active link, or any highlighted link representing which section of the page you are in or you just jumped in right now!

Solution: For the solution i have tried to change the Link tag of the code to Navlink and add on the active and pending classes as defined by the Remix docs!

Task: The problem i am stuck at, is could not find in the main css file where i can update the active link same as the hover effect that is provided in the website, so to keep the change in link ui same in hover and clicked stage.

image

brookslybrand commented 6 months ago

Hey @Sarvesh-Ghildiyal, sorry it's taken me a bit to look at this.

To answer your question, we use tailwindcss, so you wouldn't need to add class to a css file, you would add tailwind classes to get the style you want.

However, there's a bigger problem here that I just realized. To highlight these links, we need to compare the url hash. This only changes on click, which means we would only be styling it if the user clicks.

https://github.com/remix-run/remix-website/assets/12396812/9c012701-36ca-42c1-96c5-18c498fdbedb

While this is kind of nice, I don't like the inconsistent behavior with no highlighting while scrolling, and I don't really want to auto-update the hash on scroll

With all that being said I think I'm going to close this PR for now and not pursue this further at the moment.

An alternative approach would be to use intersection observers, but I think that would be even more complex, especially given how the markdown for the docs is rendered as strings and then passed into dangerouslySetInnerHTML