-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix: Improve icon contrast and text hierarchy Contributing and About Pages #4983
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
base: main
Are you sure you want to change the base?
Conversation
<div class="sm:col-span-10 text-center sm:text-left space-y-3"> | ||
<h3 class="pt-10 sm:pt-0 text-lg font-semibold"> | ||
<div class="sm:col-span-10 text-center sm:text-left"> | ||
<h3 class="text-2xl font-semibold mb-2 text-gray-800 dark:text-white"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like most of these changes, but the font size being bigger than the section title is throwing off the hierarchy:
<h3 class="text-2xl font-semibold mb-2 text-gray-800 dark:text-white"> | |
<h3 class="text-lg font-semibold mb-2 text-gray-800 dark:text-white"> |
<div class="flex sm:col-span-2 justify-center "> | ||
<%= image_tag @belief[:image_path], alt: '', class: 'w-20 h-20' %> | ||
<div class="flex sm:col-span-2 justify-center"> | ||
<div class="w-20 h-20 flex items-center justify-center rounded-full bg-gray-700 dark:bg-white shadow-md"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backgrounds being different from the surrounding background is drawing the eye a bit too much. You can test that by reading the belief content and noticing where your eyes want to go to. I think this will distract users from the main content.
What issues did you find with the contrast of these icons? They appear fine to me, and lighthouse isn't raising anything about em.
Hey @michellecaii, just checking in, do you want to finish this PR? |
Because
The About page's belief section currently uses low-contrast icons and tight typography, especially in dark mode. This affects both accessibility and visual engagement.
This PR
Updates icon styling to use high-contrast background and foreground colors for better accessibility in both light and dark modes
Enlarges icons and wraps them in rounded backgrounds to make them more visually prominent
Improves heading size and spacing for clearer text hierarchy
Issue
Additional Information
Pull Request Requirements
keyword: brief description of change
format, using one of the following keywords:Feature
- adds new or amends existing user-facing behaviorChore
- changes that have no user-facing value, refactors, dependency bumps, etcFix
- bug fixesBecause
section summarizes the reason for this PRThis PR
section has a bullet point list describing the changes in this PRIssue
section