Skip to content

Conversation

@avivkeller
Copy link
Member

Description

When the inherited text is white, the code element does not have a readable contrast, as shown below:
image

Validation

Everything on the website should look identical, but this should make a readable contrast on api-docs-tooling

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

Copilot AI review requested due to automatic review settings June 24, 2025 15:54
@avivkeller avivkeller requested a review from a team as a code owner June 24, 2025 15:54
@vercel
Copy link

vercel bot commented Jun 24, 2025

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

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jun 24, 2025 5:08pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the reliance on inherited text color for inline code elements to prevent unreadable white text in light contexts.

  • Eliminates the text-inherit utility on code within markdown styles to allow explicit coloring.
  • Ensures code blocks can receive their own color rules rather than inheriting from parent.
Comments suppressed due to low confidence (1)

packages/ui-components/styles/markdown.css:92

  • Removing the inherited text color means code elements may not have sufficient contrast by default; consider adding an explicit color utility (e.g., text-neutral-900 dark:text-neutral-100) to ensure readability across themes.
        dark:max-xs:decoration-neutral-200;

@codecov
Copy link

codecov bot commented Jun 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.46%. Comparing base (65f87ed) to head (1cf2353).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7890      +/-   ##
==========================================
- Coverage   75.47%   75.46%   -0.02%     
==========================================
  Files         101      101              
  Lines        8311     8311              
  Branches      218      218              
==========================================
- Hits         6273     6272       -1     
- Misses       2036     2037       +1     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avivkeller avivkeller changed the title fix(code): don't rely on inherited text color fix(code): don't rely on inherited text color in AlertBox Jun 24, 2025
@avivkeller avivkeller marked this pull request as draft June 24, 2025 16:20
@avivkeller avivkeller changed the title fix(code): don't rely on inherited text color in AlertBox fix(code): don't always rely on inherited text color Jun 24, 2025
@avivkeller avivkeller changed the title fix(code): don't always rely on inherited text color fix(AlertBox): don't override a Jun 24, 2025
@avivkeller avivkeller marked this pull request as ready for review June 24, 2025 16:41
@avivkeller avivkeller requested review from canerakdas and ovflowd June 24, 2025 16:41
@avivkeller avivkeller changed the title fix(AlertBox): don't override a fix(AlertBox): don't change a > code Jun 24, 2025
@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jun 24, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jun 24, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 96 🟢 100 🟠 82 🔗
/en/about/previous-releases 🟢 99 🟢 96 🟢 100 🟠 83 🔗
/en/download 🟢 97 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 100 🟢 100 🟢 96 🟢 92 🔗

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM!

@avivkeller
Copy link
Member Author

avivkeller commented Jun 24, 2025

Would you prefer?:
image


So basically ignore the markdown.css styles in an AlertBox when they conflict?

@ovflowd
Copy link
Member

ovflowd commented Jun 24, 2025

Would you prefer?: image

So basically ignore the markdown.css styles in an AlertBox when they conflict?

I think that'd make more sense to respect the style of the component.

@avivkeller avivkeller changed the title fix(AlertBox): don't change a > code fix(AlertBox): override code styles in a markdown context Jun 24, 2025
@avivkeller
Copy link
Member Author

In the Figma, there isn't a font change in <code/>, but for this, I used ibm-plex-mono, which is used in the other code blocks.

@avivkeller avivkeller added this pull request to the merge queue Jun 26, 2025
Merged via the queue into main with commit 49d6584 Jun 26, 2025
18 of 19 checks passed
@avivkeller avivkeller deleted the fix/code-inherit branch June 26, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants