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: Portal compat should apply focus-visible ponyfill #24712

Merged
merged 7 commits into from
Sep 8, 2022

Conversation

ling1726
Copy link
Member

@ling1726 ling1726 commented Sep 8, 2022

@fluentui/react-portal-compat now takes a dependency on @fluentui/react-tabster to apply the focus-vislble ponyfill. Also setup e2e tests for the portal compat package to test that the ponyfill is correctly applied.

Current Behavior

Portal compat will not apply v9 focus outline styles

New Behavior

Portal compat will apply v9 focus outline styles

Related Issue(s)

Fixes #24411

`@fluentui/react-portal-compat` now takes a dependency on
`@fluentui/react-tabster` to apply the `focus-vislble` ponyfill. Also
setup e2e tests for the portal compat package to test that the ponyfill
is correctly applied.
@fabricteam
Copy link
Collaborator

fabricteam commented Sep 8, 2022

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-portal-compat
PortalCompatProvider
0 B
0 B
5.851 kB
1.964 kB
🆕 New entry
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-accordion
Accordion (including children components)
78.914 kB
24.06 kB
react-alert
Alert
83.228 kB
20.889 kB
react-avatar
Avatar
48.381 kB
13.696 kB
react-avatar
AvatarGroup
14.95 kB
5.989 kB
react-avatar
AvatarGroupItem
68.349 kB
19.039 kB
react-button
Button
35.836 kB
9.59 kB
react-button
CompoundButton
42.862 kB
10.808 kB
react-button
MenuButton
38.454 kB
10.461 kB
react-button
SplitButton
45.87 kB
11.811 kB
react-button
ToggleButton
51.017 kB
11.007 kB
react-card
Card - All
67.002 kB
19.261 kB
react-card
Card
62.684 kB
18.177 kB
react-card
CardFooter
8.561 kB
3.601 kB
react-card
CardHeader
9.604 kB
3.94 kB
react-card
CardPreview
8.662 kB
3.656 kB
react-combobox
Combobox (including child components)
73.863 kB
24.018 kB
react-combobox
Dropdown (including child components)
73.45 kB
23.927 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
187.656 kB
51.96 kB
react-components
react-components: FluentProvider & webLightTheme
33.359 kB
11.004 kB
react-dialog
Dialog (including children components)
80.983 kB
24.186 kB
react-link
Link
12.254 kB
4.956 kB
react-menu
Menu (including children components)
115.735 kB
35.419 kB
react-menu
Menu (including selectable components)
118.934 kB
35.916 kB
react-popover
Popover
102.938 kB
31.542 kB
react-portal
Portal
10.576 kB
3.875 kB
react-provider
FluentProvider
15.755 kB
5.883 kB
react-radio
Radio
35.56 kB
11.929 kB
react-radio
RadioGroup
14.248 kB
5.7 kB
react-slider
Slider
31.526 kB
10.046 kB
react-switch
Switch
32.097 kB
10.27 kB
react-tooltip
Tooltip
41.502 kB
14.623 kB
🤖 This report was generated against e4d50f2c501c4a8be23b24373242f650a601fdaf

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 8, 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 263e12a:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration
hardcore-sunset-3jlvkq Issue #24411
elated-chatelet-9iftc0 Issue #24411

@size-auditor
Copy link

size-auditor bot commented Sep 8, 2022

Asset size changes

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

Baseline commit: e4d50f2c501c4a8be23b24373242f650a601fdaf (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 8, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1604 1635 5000
Button mount 1174 1185 5000
FluentProvider mount 1992 1949 5000
FluentProviderWithTheme mount 803 801 10
FluentProviderWithTheme virtual-rerender 726 722 10
FluentProviderWithTheme virtual-rerender-with-unmount 800 772 10
MakeStyles mount 2392 2324 50000
SpinButton mount 3105 3102 5000

@layershifter
Copy link
Member

@ling1726 Can you please add a bundle size fixture to see how it affects bundle size?

@ling1726
Copy link
Member Author

ling1726 commented Sep 8, 2022

@ling1726 Can you please add a bundle size fixture to see how it affects bundle size?

Added in 263e12a

The minified size is ~5kb. I looked into the output and the extra code is keyborg + the ponyfill itself.

It should be fine since if users are using the FluentProvider then they have already paid the cost of keyborg

@ling1726 ling1726 merged commit f1f4ab4 into microsoft:master Sep 8, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 9, 2022
* master: (63 commits)
  feat: add helper types to assist DOM element handling (microsoft#24722)
  applying package updates
  Textarea/hc bug (microsoft#24701)
  Fix null ref in use slider (microsoft#24728)
  Add Field unit tests, and remove snapshot tests (microsoft#24706)
  Stress Test: add build commands (microsoft#24575)
  Coachmark - useOffsetHeight should cause re-render on each set state to match v7 functionality (microsoft#24702)
  Implement screener tests for Field components (microsoft#24684)
  Update Field types to clean up react-field.api.md (microsoft#24703)
  fix(Popup): remove rotate(360deg) from PopupContent content styles (microsoft#24432)
  fix(FocusZone): should reset tabindex when focus is outside the zone with prop `shouldResetActiveElementWhenTabFromZone` (microsoft#24463)
  Fix greyed out legend key contrast ratio (microsoft#24714)
  fix: Portal compat should apply `focus-visible` ponyfill (microsoft#24712)
  Fix artifact error (microsoft#24717)
  chore(react-dialog): remove localShorthands in favor of griffel shorthands (microsoft#24715)
  Skip screener checks for draft PRs with exception of appropriately la… (microsoft#24694)
  fix: Remove provider classname from focus styles (microsoft#24710)
  feat: autocontrolled `useTable` hook (microsoft#24688)
  feat: add dialog properties to getNativeElementProps (microsoft#24698)
  Using migrate rather than upgrade term (microsoft#24695)
  ...
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.

[Bug]: Focus ring broken with portal compat
6 participants