Skip to content

fix(AlertBox): override code styles in a markdown context #7890

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

Merged
merged 2 commits into from
Jun 26, 2025

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 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
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 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;

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
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.

4 participants