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

feat(react-tabs): add rounded tab appearance variants #32944

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

Conversation

dmytrokirpa
Copy link
Contributor

@dmytrokirpa dmytrokirpa commented Sep 30, 2024

New Behavior

Adds a new appearance="subtle-roudned" and appearance="filled-rounded" variants to TabList and Tab components.

Screen.Recording.2024-10-01.at.10.44.45.mov

Figma - https://www.figma.com/design/dK5AnDvvnSTWV9lduQWeDk/TabList?node-id=11485-46208&t=mGEm2A0P5LNHTYUs-1

Related Issue(s)

@dmytrokirpa dmytrokirpa self-assigned this Sep 30, 2024
@fabricteam
Copy link
Collaborator

fabricteam commented Sep 30, 2024

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.1 MB
272.081 kB
1.106 MB
273.185 kB
5.951 kB
1.104 kB
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
69.14 kB
20.137 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
218.317 kB
63.258 kB
react-components
react-components: FluentProvider & webLightTheme
44.447 kB
14.59 kB
react-portal-compat
PortalCompatProvider
8.39 kB
2.64 kB
react-timepicker-compat
TimePicker
107.387 kB
35.758 kB
🤖 This report was generated against 9636aa941221c3f124f14e99939fe2d94312a086

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 30, 2024

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 41 41 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 73 78 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 637 646 5000
Button mount 299 302 5000
Field mount 1190 1167 5000
FluentProvider mount 715 722 5000
FluentProviderWithTheme mount 82 87 10
FluentProviderWithTheme virtual-rerender 41 41 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 73 78 10 Possible regression
MakeStyles mount 873 874 50000
Persona mount 1787 1744 5000
SpinButton mount 1407 1379 5000
SwatchPicker mount 1656 1655 5000

@@ -206,3 +206,31 @@ export const WithIconOnlyAndVertical = () => (
);
Copy link
Collaborator

@fabricteam fabricteam Sep 30, 2024

Choose a reason for hiding this comment

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

🕵🏾‍♀️ visual regressions to review in the fluentuiv9 Visual Regression Report

Avatar Converged 2 screenshots
Image Name Diff(in Pixels) Image Type
Avatar Converged.badgeMask.chromium.png 9 Changed
Avatar Converged.Badge Mask RTL.chromium.png 13 Changed
Drawer 3 screenshots
Image Name Diff(in Pixels) Image Type
Drawer.Full Overlay Dark Mode.chromium.png 992 Changed
Drawer.overlay drawer full.chromium.png 1139 Changed
Drawer.Full Overlay High Contrast.chromium.png 4940 Changed
TabList and Tab Converged 6 screenshots
Image Name Diff(in Pixels) Image Type
TabList and Tab Converged.Filled Rounded Appearance Dark Mode.chromium.png 0 Added
TabList and Tab Converged.Filled Rounded Appearance High Contrast.chromium.png 0 Added
TabList and Tab Converged.Filled Rounded Appearance.chromium.png 0 Added
TabList and Tab Converged.Subtle Rounded Appearance Dark Mode.chromium.png 0 Added
TabList and Tab Converged.Subtle Rounded Appearance High Contrast.chromium.png 0 Added
TabList and Tab Converged.Subtle Rounded Appearance.chromium.png 0 Added

@dmytrokirpa dmytrokirpa marked this pull request as ready for review October 1, 2024 08:57
@dmytrokirpa dmytrokirpa requested review from a team as code owners October 1, 2024 08:57
Copy link
Contributor

@mainframev mainframev left a comment

Choose a reason for hiding this comment

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

LGTM, few small suggestions

Copy link
Contributor

@Hotell Hotell left a comment

Choose a reason for hiding this comment

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

the api design is miss-leading ( not referring to your PR -> we need to talk to design ) more context #32951 (comment)

in terms of actual api implementation, we should not introduce another property rather provide discriminant union of variants. any kind of new property addition = hard to maintain property bag

@dmytrokirpa
Copy link
Contributor Author

the api design is miss-leading ( not referring to your PR -> we need to talk to design ) more context #32951 (comment)

in terms of actual api implementation, we should not introduce another property rather provide discriminant union of variants. any kind of new property addition = hard to maintain property bag

I appreciate the feedback, updated PR to use existing prop with new variants.

@dmytrokirpa dmytrokirpa changed the title feat(react-tabs): add circular tab variant feat(react-tabs): add rounded tab variant Oct 3, 2024
@dmytrokirpa dmytrokirpa changed the title feat(react-tabs): add rounded tab variant feat(react-tabs): add rounded tab appearance variants Oct 3, 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.

4 participants