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(web-components): fix disabled button styles #31585

Merged

Conversation

davatron5000
Copy link
Contributor

@davatron5000 davatron5000 commented Jun 5, 2024

Previous Behavior

  • Bug filed for [disabled][appearance="subtle"] fluent-buttons where they are expected to not have a background or outline when disabled, but actually were showing a background and outline.
    • Confirmed that a "fix" in fa9416a caused a regression where "fixed" styles were now overriding existing behavior due to cascade order.
    • This exposed a false positive in our Playwright tests where CSS variables were being wiped out by the use of setContent.
  • Discovered a "focusabale" typo in button styles
  • Discovered a bug with :focus-visible not working in High Contrast Mode

New Behavior

  • Reorganized CSS so that [disabled] styles cascade correctly.
  • Fixed focusabale typo
  • Removed errant ) char from :focus-visible styles to fix high-contrast mode focus states.
  • Thanks to help from @radium-v and prior investigation by @janechu, changed how we register CSS properties in setTheme to prevent them being wiped out by page.setContent
    • Now using CSS.registerProperty with a fallback to current method for current versions of Firefox and older browsers.
  • Fixed and improved tests to help preserve behavior.

The CSS.registerProperty change is probably significant enough to heavily scrutinize this PR. It's pretty naive at the moment, just applying an initialValue. In the future I think we'd want to set actual CSSOM types (integer, pixel values, etc).

Look forward to feedback.

Related Issue(s)

Screenshots

Screenshot 2024-06-05 at 11 34 42 AM

Screenshot 2024-06-05 at 11 36 00 AM

@davatron5000 davatron5000 requested a review from a team as a code owner June 5, 2024 16:37
@fabricteam
Copy link
Collaborator

fabricteam commented Jun 5, 2024

📊 Bundle size report

✅ No changes found

Copy link

codesandbox-ci bot commented Jun 5, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@davatron5000 davatron5000 force-pushed the davatron5000/fix-disabled-buttons branch from 05baa54 to 2884fe7 Compare June 5, 2024 17:15
Copy link

codesandbox-ci bot commented Jun 5, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@chrisdholt
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

Copy link

codesandbox-ci bot commented Jun 5, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@chrisdholt chrisdholt merged commit 5987b68 into microsoft:master Jun 5, 2024
18 of 19 checks passed
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
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.

Disabled styles on subtle buttons is not applied correctly
4 participants