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

Guides Dark Mode fixes #51444

Merged
merged 3 commits into from Mar 30, 2024
Merged

Guides Dark Mode fixes #51444

merged 3 commits into from Mar 30, 2024

Conversation

jathayde
Copy link
Contributor

@jathayde jathayde commented Mar 29, 2024

Screenshot 2024-03-28 at 9 38 25 PM

Motivation / Background

This is a final (?) PR related to feedback on dark mode from various users and DHH/Core team members:

Referencing @claudioduarte in #51415

  • Make alt, shorter SVG line for mobile, add responsive style
  • Fix padding on footer (goes to edge on mobile, does not align on desktop)
  • Shrink overall height of subCol/sidebar so it doesn’t overlap footer

Referencing @ivanjuric in #51398

  • Flip inverted dark modes on “more Ruby on Rails” nav bar in mobile view

Notes from DHH & Core Team:

  • Darker dark mode (Github non-dimmed style)
  • Remove double box on WIP call out on index right side
  • NavBar bottom issue clipping on some guides (pad the content?)
  • Rounded corners/better appearance on select form elements in header
  • Black border on bottom of interstitials
  • Font size to 16px (100%) from 18px base
  • Brighten syntax highlighting across the board

Additional information

Screenshot 2024-03-28 at 9 38 36 PM Screenshot 2024-03-28 at 9 39 29 PM

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@claudioduarte in rails#51415

- [x] Make alt, shorter SVG line for mobile, add responsive style
- [x] Add side padding on footer (goes to edge on mobile, does not align on desktop)
- [x] Shrink overall height of subCol so it doesn’t overlap footer

@ivanjuric in rails#51398

- [x] Flip inverted dark modes on “more Ruby on Rails” nav bar

DHH/Core:

- [x] Darker dark mode (Github style)
- [x] Double box on WIP call out (index.html. editable?)
- [x] NavBar bottom issue clipping on some guides (pad the content?)
- [x] Rounded corners/better appearance on select form elements in header
- [x] Black border on bottom of interstitials
- [x] tightening up white space, adding Rails brand HR after intro
- [x] base fontsize to 16px from 18px;
- [x] fixing some darker dark mode issues in the last commit
@rails-bot rails-bot bot added the docs label Mar 29, 2024
@AmandaPerino AmandaPerino requested a review from dhh March 29, 2024 12:00
@AmandaPerino AmandaPerino added the rails foundation Rails Foundation PRs label Mar 29, 2024
@lloydk
Copy link
Contributor

lloydk commented Mar 29, 2024

To much contrast

I highly recommend lowering the contrast of the text in dark mode. To much contrast in dark mode can cause halation and makes the content hard to read. I think that lowering the contrast will result in a more pleasant reading experience.

The Accessible Perceptual Contrast Algorithm (APCA) has some proposed guidelines for maximum contrast for dark mode.

Proposed changes based on the APCA guidelines:

Text content should have a maximum contrast of Lc 90 and text >= 36px or bold text >= 24px (h1, h2, h3) should have a maximum contrast of Lc 75.

Background of #181818

Background of #292929 (Chapter guide)

  • Can use the same colors that were used for a background of #181818.

Note that I'm not proposing any changes to the colors to the sample code.

Red/Pink links

I'll preface this by pointing out that made the same changes to pass the WCAG standard.

Readability should trump branding especially for documentation.

I understand the desire to use red links for branding purposes and the red link color passes the WCAG standard (barely) but the links have a very low contrast compared to the rest of the text which is distracting and makes the content harder to read.

The red links have a very low APCA score Lc 44.6 which is very far from the minimum recommendation of Lc 75 for body text.

A random color that I picked #94e0fa which is Lc 80.1 or any color that gives an APCA Lc score between 75-90 will provide a better reading experience.

The hover color for the chapter links should also be changed to something more readable.

Other suggestions

Change the background of body.guide p code, ... to #303030 and the color to #e5e5e5. I think the difference between the background colors of #181818 and #505050 is distracting and a subtle background difference would be more readable.

Lower the contrast and possibly change the color of the Chapters section to provide more "contrast" with the main content. APCA suggests a preferred contrast of Lc75 for primary navigation. For example the random blue I picked above (#94e0fa) would have an Lc of 77.6 against the background of #292929.

From APCA Use Cases and Conformance Research

Fluent Readability, Primary content that is not body text
up to two lines of continuous text (per the normal non-zoomed layout)
No larger than an x-height of 80px (Verdana 144px or Times 178px)
No smaller than an x-height of 8px (Verdana 14.4px or Times 17.8px)
MINIMUM weight: 300 for fonts under 24px (x-height 12)
Minimum weight 200 for fonts above 24px.
MINIMUM contrast of Lc 60
PREFERRED contrast: Lc 75
includes headlines, captions, and images of text, if they contribute to content
primary navigation and primary menus
asides, tool-tips, spoilers, "continued"
user entered text in forms, fields, etc

I think this screenshot contains most of the suggested changes

image

@dhh dhh merged commit ee4a371 into rails:main Mar 30, 2024
4 checks passed
@dhh
Copy link
Member

dhh commented Mar 30, 2024

I'm fine with the background color and text color from the comment above, but we'll stick to the red for the links. If there's slight tweak to that red that would improve things, I'm happy to consider that too. But we're not introducing a new off-brand color for this.

@lloydk
Copy link
Contributor

lloydk commented Mar 30, 2024

I'll create a PR for the background color and text color changes.

For the links how about something like this:

image

I've taken the logo red and lightened it until it passes APCA Lc 75 which is the minimum for body text and preferred APCA Lc for primary navigation.

Here's a link inside a paragraph with along with the text that has a lower contrast as suggested above.

image

@dhh
Copy link
Member

dhh commented Mar 31, 2024

Red in the sidebar is a no go. And the one in the main text looks rather sickly to me. But I'm good with background and the new white.

Choose a reason for hiding this comment

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

@jathayde did you mean to commit the css files here again? They were being generated to guides/output when we first introduced them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, they should not be here.

Choose a reason for hiding this comment

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

Ok thanks, I've pushed a commit removing them: 0c1804d

@SleeplessByte SleeplessByte mentioned this pull request Apr 5, 2024
4 tasks
fractaledmind pushed a commit to fractaledmind/rails that referenced this pull request May 13, 2024
* Guides feedback fixes

@claudioduarte in rails#51415

- [x] Make alt, shorter SVG line for mobile, add responsive style
- [x] Add side padding on footer (goes to edge on mobile, does not align on desktop)
- [x] Shrink overall height of subCol so it doesn’t overlap footer

@ivanjuric in rails#51398

- [x] Flip inverted dark modes on “more Ruby on Rails” nav bar

DHH/Core:

- [x] Darker dark mode (Github style)
- [x] Double box on WIP call out (index.html. editable?)
- [x] NavBar bottom issue clipping on some guides (pad the content?)
- [x] Rounded corners/better appearance on select form elements in header
- [x] Black border on bottom of interstitials

* Additional UI cleanup

- [x] tightening up white space, adding Rails brand HR after intro
- [x] base fontsize to 16px from 18px;
- [x] fixing some darker dark mode issues in the last commit

* Lightening syntax highlight colors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs rails foundation Rails Foundation PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants