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

chore(react-tree): adds test to ensure action visibility on click #32042

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bsunderhus
Copy link
Contributor

@bsunderhus bsunderhus commented Jul 18, 2024

Previous Behavior

We are assuming navigation on click events (to ensure proper tabIndex on the clicked treeitem), as described here #32034 (comment), this assumption is not correct, we should be doing that on focus events!

New Behavior

  1. removes navigation from click events
  2. adds focus handler to navigate on focus events
  3. adds test to ensure actions slot are visible on a real click
  4. deprecates TreeNavigationData union with the Click event type
  5. introduces Focus event type for the TreeNavigationData union

Related Issue(s)

@bsunderhus bsunderhus self-assigned this Jul 18, 2024
@github-actions github-actions bot added this to the July Project Cycle Q3 2024 milestone Jul 18, 2024
@fabricteam
Copy link
Collaborator

fabricteam commented Jul 18, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.096 MB
271.19 kB
1.096 MB
271.217 kB
69 B
27 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
69.141 kB
20.157 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
211.781 kB
60.963 kB
react-components
react-components: FluentProvider & webLightTheme
44.442 kB
14.607 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
react-timepicker-compat
TimePicker
106.611 kB
35.546 kB
🤖 This report was generated against 705085a130e977bbddd81582e201fd218652a536

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 18, 2024

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 651 652 5000
Button mount 303 316 5000
Field mount 1140 1144 5000
FluentProvider mount 731 717 5000
FluentProviderWithTheme mount 97 108 10
FluentProviderWithTheme virtual-rerender 40 42 10
FluentProviderWithTheme virtual-rerender-with-unmount 87 96 10
MakeStyles mount 875 885 50000
Persona mount 1784 1736 5000
SpinButton mount 1446 1440 5000
SwatchPicker mount 1687 1683 5000

@bsunderhus bsunderhus marked this pull request as ready for review July 18, 2024 08:05
@bsunderhus bsunderhus requested a review from a team as a code owner July 18, 2024 08:05
@bsunderhus bsunderhus force-pushed the react-tree/chore--adds-test-to-ensure-action-visibility-on-click branch from 39e29dd to c0ade1d Compare July 18, 2024 08:48
@bsunderhus bsunderhus force-pushed the react-tree/chore--adds-test-to-ensure-action-visibility-on-click branch from c0ade1d to 69bc973 Compare July 18, 2024 10:21
@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

@@ -20,6 +20,13 @@ export type TreeNavigationData_unstable = {
value: TreeItemValue;
parentValue: TreeItemValue | undefined;
} & (
| { event: React.FocusEvent<HTMLElement>; type: 'Focus' }
Copy link
Member

Choose a reason for hiding this comment

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

isn't this a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is. Any suggestions to avoid it?

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.

[Bug]: TreeItem click events firing navigation
3 participants