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

fix(a11y): fixed color contrast and heading issues #7424

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lyannel
Copy link

@lyannel lyannel commented Jan 10, 2025

Changes

  • Fixed color contrast and heading a11y issues on Learn Page
  • Fixed 90% of a11y issues on Learn Page

Screenshots

Before Changes

Before A11y changes

After Changes

A11y changes

Copy link

vercel bot commented Jan 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
19-react-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 9:41pm
react-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 10, 2025 9:41pm

Copy link

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

🎉 Global Bundle Size Decreased

Page Size (compressed)
global 104.37 KB (-9 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Five Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/404 105.95 KB (🟢 -12 B) 210.32 KB
/500 105.95 KB (🟢 -12 B) 210.32 KB
/[[...markdownPath]] 107.74 KB (🟢 -12 B) 212.11 KB
/errors 106.16 KB (🟢 -12 B) 210.53 KB
/errors/[errorCode] 106.14 KB (🟢 -12 B) 210.51 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 10% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

Thanks for taking a pass at this!

@@ -66,7 +66,7 @@ export function SidebarLink({
'text-primary dark:text-primary-dark': level === 0 && !selected,
'text-base text-secondary dark:text-secondary-dark':
level > 0 && !selected,
'text-base text-link dark:text-link-dark bg-highlight dark:bg-highlight-dark border-blue-40 hover:bg-highlight hover:text-link dark:hover:bg-highlight-dark dark:hover:text-link-dark':
'text-base text-primary dark:text-link-dark bg-highlight dark:bg-highlight-dark border-blue-40 hover:bg-highlight hover:text-link dark:hover:bg-highlight-dark dark:hover:text-link-dark':
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using text-primary can we add test-link-secondary with value #045975 (blue-60). This will keep the vibe, while maintaining a good contrast ratio.

@@ -129,7 +129,7 @@ function NavItem({url, isActive, children}: any) {
'active:scale-95 transition-transform w-full text-center outline-link py-1.5 px-1.5 xs:px-3 sm:px-4 rounded-full capitalize whitespace-nowrap',
!isActive && 'hover:bg-primary/5 hover:dark:bg-primary-dark/5',
isActive &&
'bg-highlight dark:bg-highlight-dark text-link dark:text-link-dark'
'bg-highlight dark:bg-highlight-dark text-link-10 dark:text-link-dark'
Copy link
Member

Choose a reason for hiding this comment

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

I don't think text-link-10 is set to anything? Can we use test-link-secondary from above?

@@ -304,7 +304,7 @@ export default function TopNav({
<button
type="button"
className={cn(
'flex 3xl:w-[56rem] 3xl:mx-0 relative ps-4 pe-1 py-1 h-10 bg-gray-30/20 dark:bg-gray-40/20 outline-none focus:outline-link betterhover:hover:bg-opacity-80 pointer items-center text-start w-full text-gray-30 rounded-full align-middle text-base'
'flex 3xl:w-[56rem] 3xl:mx-0 relative ps-4 pe-1 py-1 h-10 bg-gray-30/20 dark:bg-gray-40/20 outline-none focus:outline-link betterhover:hover:bg-opacity-80 pointer items-center text-start w-full rounded-full align-middle text-base'
Copy link
Member

Choose a reason for hiding this comment

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

I believe test-gray-50 will give enough contrast.

@@ -18,7 +18,7 @@ function InlineCode({
className={cn(
'inline text-code text-secondary dark:text-secondary-dark px-1 rounded-md no-underline',
{
'bg-gray-30 bg-opacity-10 py-px': !isLink,
'bg-opacity-10 py-px': !isLink,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of removing the background, can you update the a > code color to be #045975 (blue-60) here instead:

a > code {
color: #087ea4 !important; /* blue-50 */
text-decoration: none !important;
}

@@ -263,7 +263,7 @@ export default function ShoppingList() {
<li
key={product.id}
style={{
color: product.isFruit ? 'magenta' : 'darkgreen'
color: product.isFruit ? 'darkblue' : 'darkgreen'
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a different color or change the magenta color? The tool doesn't know that these items need to have good contrast relative to each other, otherwise it's hard to see the point it's making:

Screenshot 2025-02-18 at 12 32 32 AM

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

Successfully merging this pull request may close these issues.

3 participants