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

(web components) Standardize focus treatment #24771

Merged
merged 4 commits into from
Sep 16, 2022

Conversation

bheston
Copy link
Contributor

@bheston bheston commented Sep 12, 2022

Current Behavior

In the web component styles, the focus treatment is a mix of reusing existing display borders or supplementing borders with inset box-shadows. This is difficult to style and creates inconsistencies between components, states, and is blocking the update to support color opacity for use over mica.

New Behavior

Since outline is fully support and now follows border-radius, it's a much better option for a treatment that can overlay and ignore any potential border treatments. By separating the focus treatment from any visible borders, we can more easily and consistently apply the opacity color update.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 12, 2022

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.

Latest deployment of this branch, based on commit 618d27e:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Sep 12, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 7c868d1d8a5f0e158ebda2b42d9dba486f72053a (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

📊 Bundle size report

🤖 This report was generated against b7395d67b12bc51b0785a022a4a60930d7543d3c

@Hotell Hotell removed their assignment Sep 13, 2022
@EisenbergEffect
Copy link
Contributor

EisenbergEffect commented Sep 15, 2022

I'm not sure how we should treat this PR. Can you provide a couple of before and after screenshots? I'm trying to understand if this actually constitutes a significant visual change to the component focus styles or if it's more subtle (and a flexibility improvement). The reason is that if it's not a minor visual change that isn't going to mess folks up, then we need to bump the major version. And I don't think we should bump a major version while we're currently in progress on the next major version.

In general, I don't think we should be spending any time on this version of Fluent unless it's to fix critical bugs because we need to get the new Fluent implementation up and running pretty soon so we don't cause major blocks for other teams.

@bheston
Copy link
Contributor Author

bheston commented Sep 16, 2022

I'm not sure how we should treat this PR. Can you provide a couple of before and after screenshots? I'm trying to understand if this actually constitutes a significant visual change to the component focus styles or if it's more subtle (and a flexibility improvement).

This was not intended to be a visual change in any way. I've gone back through and created before and after screenshots, which illustrate only one minor difference on the offset of the treatment on the breadcrumb, which is desirable, and I don't believe breaking.

I fixed a few unintended visual changes, so hopefully there are no concerns merging this.

Before
Before

Update
Update

@EisenbergEffect
Copy link
Contributor

Awesome! Just wanted to make sure. From reading the CSS it didn't look like it was a real visual change. The screenshots help a lot though.

@chrisdholt chrisdholt merged commit 2420757 into microsoft:master Sep 16, 2022
@bheston bheston deleted the update-focus-treatment branch September 16, 2022 21:16
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 19, 2022
* master: (28 commits)
  fix: use trigger prop for aria-haspopup (microsoft#24794)
  chore(react-dialog): scaffold DialogContent component (microsoft#24844)
  chore: Northstar screener should read from screenerStates.json (microsoft#24848)
  applying package updates
  (web components) Standardize focus treatment (microsoft#24771)
  Divider - allow default prop override (microsoft#24840)
  GroupedList: fix virtualization (unstable preview) (microsoft#24460)
  fix: Add explicit children prop to TeachingBubble to support React 18 (microsoft#24823)
  feat: Adds `visible` prop to `TableCellActions` (microsoft#24831)
  [Northstar][Dropdown] Fix styling mutation when merging themes (microsoft#24787)
  fix: export `tableCellActionsClassNames` from unstable (microsoft#24830)
  bugfix(react-dialog): Adds color style to DialogSurface (microsoft#24832)
  applying package updates
  Prevent group toggling from selecting the whole group (microsoft#24822)
  feat(react-textarea): Add shadow variant of filled appearance (microsoft#24512)
  applying package updates
  Adding lib-commonjs top-level entries to exports map (microsoft#24792)
  Created shim packages (microsoft#24780)
  feat(react-menu): replace keydown handlers by useARIAButtonShorthand on MenuItem (microsoft#24738)
  fix: update version mismatches triggered by v9 release (microsoft#24812)
  ...
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.

6 participants