Skip to content

UI: Remove background for dark background active Badge to increase contrast#33065

Closed
MichaelArestad wants to merge 2 commits into
nextfrom
m/update-active-badge-background
Closed

UI: Remove background for dark background active Badge to increase contrast#33065
MichaelArestad wants to merge 2 commits into
nextfrom
m/update-active-badge-background

Conversation

@MichaelArestad
Copy link
Copy Markdown
Contributor

@MichaelArestad MichaelArestad commented Nov 17, 2025

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Style
    • Updated Badge component styling to provide better visual consistency across theme variations.

@MichaelArestad MichaelArestad self-assigned this Nov 17, 2025
@MichaelArestad MichaelArestad added maintenance User-facing maintenance tasks ci:normal labels Nov 17, 2025
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Nov 17, 2025

View your CI Pipeline Execution ↗ for commit 514e4de

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 53s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-19 15:25:50 UTC

@MichaelArestad MichaelArestad marked this pull request as ready for review November 17, 2025 22:43
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 17, 2025

📝 Walkthrough

Walkthrough

The Badge component's active status styling now conditionally sets the background based on the theme base. When the theme base is dark, the background is unset; otherwise, it retains the previous hoverable background value.

Changes

Cohort / File(s) Summary
Badge active status styling
code/core/src/components/components/Badge/Badge.tsx
Active state background now conditionally set to unset for dark theme bases, otherwise retains hoverable background

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single focused change limited to one component file
  • Straightforward conditional logic for theme-based styling
  • No public API or structural modifications
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5329d93 and d85e293.

📒 Files selected for processing (1)
  • code/core/src/components/components/Badge/Badge.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
🔇 Additional comments (1)
code/core/src/components/components/Badge/Badge.tsx (1)

79-79: LGTM! Change correctly implements the contrast improvement.

The conditional background removal on dark themes will increase contrast as intended. The boxShadow on line 80 ensures the badge boundary remains visible in both themes.


Comment @coderabbitai help to get the list of available commands and usage tips.

@MichaelArestad MichaelArestad marked this pull request as draft November 19, 2025 15:19
@MichaelArestad
Copy link
Copy Markdown
Contributor Author

If #33081 is merged, this is unnecessary.

@MichaelArestad
Copy link
Copy Markdown
Contributor Author

Closed as the status PR handled this.

@MichaelArestad MichaelArestad deleted the m/update-active-badge-background branch November 25, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:normal maintenance User-facing maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant