Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remix RTD Docs design revamp and unification #3198

Open
wants to merge 71 commits into
base: master
Choose a base branch
from

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented May 8, 2024

Description

Brings new menu navigation and color themes in tandem with unifying updates to the Remix Project homepage.

image

Preview URL

Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

@wackerow minor UI issue

The base font seems to be Helvetica, but in this case, its using Arial
image

@wackerow wackerow marked this pull request as ready for review May 15, 2024 08:58
Copy link
Member

@pettinarip pettinarip left a comment

Choose a reason for hiding this comment

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

  1. Small problem with the sidebar. Long titles cause horizontal scrolling, which I'm not sure we want. This example is in the Spanish version
    image

This scrolling is also going to break a bit the mobile experience.

  1. Probably not something related to this revamp but just noted that we have this copy where we mention that we have more translations than the ones we have on the menu
    image

  2. Nice to have: capital "E" on "espanol"
    image

Last observation, the docs are translated into 3 languages while the homepage is in 2. Spanish is the difference. Probably not a problem, just wanted to mention it in case we need consistency there.

@wackerow
Copy link
Member Author

Thanks @pettinarip! Few responses and follow-up questions:

The base font seems to be Helvetica, but in this case, its using Arial

I'm not getting this... are you still seeing this? If so, let me know what browser you're using.

  1. Small problem with the sidebar. Long titles cause horizontal scrolling, which I'm not sure we want. This example is in the Spanish version

Thanks, I'll look into and try to get a patch up.

  1. Probably not something related to this revamp but just noted that we have this copy where we mention that we have more translations than the ones we have on the menu

Yeah this would be out of scope here... would suggest the team edits that once the upgrade is complete with the latest languages.

  1. Nice to have: capital "E" on "espanol"

I'm grabbing the language codes from the "flyover" menu that generates automatically by RTD (the names don't populate, just the codes).. from there I'm feeding it to Intl.DisplayNames to generate that display name... it's browser specific, so some will produce español, others will show Español.... the only way to avoid this would be to also store a mapping of lang codes to their display names, which I avoided to make maintenance easier.

@wackerow
Copy link
Member Author

Just pushed a patch for the text-overflow... @pettinarip let me know if you're still getting the Arial bug, otherwise I think this is good for another review.

@wackerow wackerow requested a review from pettinarip May 17, 2024 10:58
@pettinarip
Copy link
Member

Just pushed a patch for the text-overflow... @pettinarip let me know if you're still getting the Arial bug, otherwise I think this is good for another review.

I see this just in Brave (Version 1.66.110 Chromium: 125.0.6422.60 (Official Build) (64-bit)). Chrome and FF are working fine.

Minor issue I'd say. Anyway, for some reason, the styles for this button are overridden by the browser's styles.

image

@pettinarip
Copy link
Member

Thanks @pettinarip! Few responses and follow-up questions:

The base font seems to be Helvetica, but in this case, its using Arial

I'm not getting this... are you still seeing this? If so, let me know what browser you're using.

  1. Small problem with the sidebar. Long titles cause horizontal scrolling, which I'm not sure we want. This example is in the Spanish version

Thanks, I'll look into and try to get a patch up.

  1. Probably not something related to this revamp but just noted that we have this copy where we mention that we have more translations than the ones we have on the menu

Yeah this would be out of scope here... would suggest the team edits that once the upgrade is complete with the latest languages.

  1. Nice to have: capital "E" on "espanol"

I'm grabbing the language codes from the "flyover" menu that generates automatically by RTD (the names don't populate, just the codes).. from there I'm feeding it to Intl.DisplayNames to generate that display name... it's browser specific, so some will produce español, others will show Español.... the only way to avoid this would be to also store a mapping of lang codes to their display names, which I avoided to make maintenance easier.

Got it. All working fine from my side now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants