Skip to content

fix: ButtonSecondary disabled-state outline in dark mode#1168

Open
Kureev wants to merge 3 commits into
MetaMask:mainfrom
Kureev:kureev/MUSD-783
Open

fix: ButtonSecondary disabled-state outline in dark mode#1168
Kureev wants to merge 3 commits into
MetaMask:mainfrom
Kureev:kureev/MUSD-783

Conversation

@Kureev

@Kureev Kureev commented May 8, 2026

Copy link
Copy Markdown

Description

In dark mode, the disabled ButtonSecondary (default variant) blends into the page background and reads as floating text rather than a control. This adds a 1px border-default outline to the disabled state when the active theme is dark. Light mode, all non-disabled states, and the isDanger / isInverse / isInverse + isDanger sub-variants are unchanged.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/MUSD-783

Manual testing steps

  1. Open Storybook (or any consumer rendering ButtonSecondary).
  2. Switch the app theme to dark mode.
  3. Render a ButtonSecondary with isDisabled={true} (e.g. the isDisabled story under ButtonSecondary).
  4. Confirm the button now shows a 1px outline using the border-default token, distinct from the page background.
  5. Switch to light mode and confirm the disabled ButtonSecondary is unchanged (no visible outline).
  6. With dark mode active, confirm non-disabled (default, pressed, loading) states still render without an outline, and the isDanger / isInverse / isInverse + isDanger sub-variants render exactly as before.

Screenshots/Recordings

BEFORE
image

AFTER
image

Pre-merge author checklist

  • I've followed MetaMask Contributor Docs
  • I've completed the PR template to the best of my ability
  • I've included tests if applicable
  • I've documented my code using JSDoc format if applicable
  • I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Low Risk
Low risk styling change limited to ButtonSecondary’s disabled default state in dark mode, with added unit tests to lock behavior. No behavioral, data, or security-sensitive logic is affected.

Overview
Fixes the disabled default ButtonSecondary appearance in dark mode by applying a border-default outline when isDisabled and the active theme is Theme.Dark (light mode and other sub-variants keep their existing borders).

Updates tests to be theme-aware via ThemeProvider, adds a helper to assert borderColor, and covers both dark-mode outlined and light-mode transparent disabled borders.

Reviewed by Cursor Bugbot for commit 2f6c455. Bugbot is set up for automated code reviews on this repo. Configure here.

Kureev added 3 commits May 8, 2026 11:39
In dark mode, the disabled `ButtonSecondary` blends into the page
background and reads as floating text. Add a 1px `border-default`
outline to the disabled state when the active theme is dark, leaving
light-mode rendering and all non-disabled states unchanged.
Adds two cases in `ButtonSecondary.test.tsx` to keep the package at
100% branch coverage after introducing the dark-mode disabled outline:

- default + `isDisabled` under `Theme.Dark` → border resolves to
  `border-default` from a dark-themed tailwind instance.
- default + `isDisabled` under the implicit light theme → border stays
  `border-transparent`.

Introduces a small `expectBorderColor` helper mirroring
`expectBackground` and a `twDark` instance produced via `renderHook`
wrapped in `<ThemeProvider theme={Theme.Dark}>`.
Adds a trailing `expect(btn).toBeDefined()` to the two new dark-mode
disabled-border tests so they match the existing pattern in this
file and pass the `jest/expect-expect` lint rule, which does not
recognise the `expectBorderColor`/`expectBackground` helpers as
assertion sites.
@Kureev Kureev marked this pull request as ready for review May 8, 2026 10:13
@Kureev Kureev requested a review from a team as a code owner May 8, 2026 10:13

@georgewrmarshall georgewrmarshall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey @Kureev, thanks for the contribution. Can you also make this update to the design-system-react version of ButtonSecondary?

I think we should also check whether we can keep the same border color for the enabled state as well as in light mode. That would reduce a lot of the complexity here.

A couple of questions:

  • Should the border only exist when the button is disabled?
  • Can we use the same border color in light mode to reduce complexity?

@brianacnguyen

Copy link
Copy Markdown
Contributor

Hey @Kureev, thanks for the contribution. Can you also make this update to the design-system-react version of ButtonSecondary?

I think we should also check whether we can keep the same border color for the enabled state as well as in light mode. That would reduce a lot of the complexity here.

A couple of questions:

  • Should the border only exist when the button is disabled?
  • Can we use the same border color in light mode to reduce complexity?

@amandaye0h can you help advice here? IMO I think the border should exist in light mode disabled as well to reduce complexity

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.

3 participants