fix: TableHeaderCell design fixes #25712

ling1726 · 2022-11-18T13:10:12Z

Sort arrow is 12px
Sort arrow has a xxs top padding for alignment
Gap between header text and sort icon is xs

Addresses [Bug]: Table visual bug bash #25706

- Sort arrow is 12px - Sort arrow has a xxs top padding for alignment - Gap between header text and sort icon is xs

size-auditor · 2022-11-18T13:22:26Z

Asset size changes

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

Baseline commit: 3951937c37df0de5d94f0f749d87030e23da1099 (build)

fabricteam · 2022-11-18T13:24:28Z

📊 Bundle size report

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-components` react-components: Button, FluentProvider & webLightTheme	`62.975 kB` `17.687 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`190.427 kB` `53.087 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.48 kB` `11.037 kB`
`react-portal-compat` PortalCompatProvider	`5.857 kB` `1.978 kB`

_{🤖 This report was generated against 3951937c37df0de5d94f0f749d87030e23da1099}

codesandbox-ci · 2022-11-18T13:26:23Z

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 3e526f0:

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

fabricteam · 2022-11-18T13:26:52Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1274	1275	5000
Button	mount	913	907	5000
FluentProvider	mount	1500	1491	5000
FluentProviderWithTheme	mount	577	579	10
FluentProviderWithTheme	virtual-rerender	551	545	10
FluentProviderWithTheme	virtual-rerender-with-unmount	577	595	10
MakeStyles	mount	1951	1963	50000
SpinButton	mount	2345	2335	5000

fabricteam · 2022-11-18T13:34:35Z

🕵 fluentuiv9 Open the Visual Regressions report to inspect the 20 screenshots

✅ There was 0 screenshots added, 0 screenshots removed, 1731 screenshots unchanged, 0 screenshots with different dimensions and 20 screenshots with visible difference.

unknown 20 screenshots

Image Name	Diff(in Pixels)	Image Type
Avatar Converged.size+active+badge.normal.chromium.png	7	Changed
Avatar Converged.size+active+ring-shadow.normal.chromium.png	3	Changed
Table layout flex - headers.sortable - Dark Mode.chromium.png	79	Changed
Table layout flex - headers.sortable - Dark Mode.hover header.chromium.png	78	Changed
Table layout flex - headers.sortable - Dark Mode.press header.chromium.png	79	Changed
Table layout flex - headers.sortable - High Contrast.chromium.png	95	Changed
Table layout flex - headers.sortable - High Contrast.hover header.chromium.png	79	Changed
Table layout flex - headers.sortable - High Contrast.press header.chromium.png	79	Changed
Table layout flex - headers.sortable.chromium.png	82	Changed
Table layout flex - headers.sortable.hover header.chromium.png	81	Changed
Table layout flex - headers.sortable.press header.chromium.png	79	Changed
Table layout table - headers.sortable - Dark Mode.chromium.png	79	Changed
Table layout table - headers.sortable - Dark Mode.hover header.chromium.png	78	Changed
Table layout table - headers.sortable - Dark Mode.press header.chromium.png	79	Changed
Table layout table - headers.sortable - High Contrast.chromium.png	95	Changed
Table layout table - headers.sortable - High Contrast.hover header.chromium.png	79	Changed
Table layout table - headers.sortable - High Contrast.press header.chromium.png	79	Changed
Table layout table - headers.sortable.chromium.png	82	Changed
Table layout table - headers.sortable.hover header.chromium.png	81	Changed
Table layout table - headers.sortable.press header.chromium.png	79	Changed

layershifter · 2022-11-22T11:02:24Z

packages/react-components/react-table/src/components/TableHeaderCell/useTableHeaderCell.tsx

+  ascending: <ArrowUpRegular fontSize={12} />,
+  descending: <ArrowDownRegular fontSize={12} />,


An icon could be overridden, correct? So...

Before

<TableHeaderCell sortIcon={<FooIcon />} />

After

<TableHeaderCell sortIcon={<FooIcon fontSize={12} />} />

Should the size be set via styles?

If an icon is overridden, then I would expect the user to take full responsibility of sizing, since at that point they are committed to using something that is not our guidance, since we explicitly provide a default icon here

fabricteam · 2022-11-22T14:00:25Z

🕵 fluentuiv8 Open the Visual Regressions report to inspect the 2 screenshots

✅ There was 0 screenshots added, 2 screenshots removed, 1037 screenshots unchanged, 0 screenshots with different dimensions and 0 screenshots with visible difference.

unknown 2 screenshots

Image Name	Diff(in Pixels)	Image Type
Pivot - Overflow.Tabs - RTL.Narrow - Last tab selected.chromium.png	0	Removed
Pivot - Overflow.Tabs - RTL.Narrow - Overflow menu.chromium.png	0	Removed

fabricteam · 2022-11-22T14:07:26Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
BaseButton	mount	1213	1214	5000
Breadcrumb	mount	2820	2831	1000
Checkbox	mount	2629	2643	5000
CheckboxBase	mount	2364	2390	5000
ChoiceGroup	mount	4305	4266	5000
ComboBox	mount	1172	1170	1000
CommandBar	mount	9337	9328	1000
ContextualMenu	mount	10291	10447	1000
DefaultButton	mount	1383	1354	5000
DetailsRow	mount	3415	3359	5000
DetailsRowFast	mount	3360	3376	5000
DetailsRowNoStyles	mount	3234	3240	5000
Dialog	mount	2981	2955	1000
DocumentCardTitle	mount	583	592	1000
Dropdown	mount	3170	3172	5000
FocusTrapZone	mount	1944	1954	5000
FocusZone	mount	1908	1955	5000
GroupedList	mount	1850	2057	2
GroupedList	virtual-rerender	1117	1094	2
GroupedList	virtual-rerender-with-unmount	1624	1620	2
GroupedListV2	mount	570	584	2
GroupedListV2	virtual-rerender	543	543	2
GroupedListV2	virtual-rerender-with-unmount	555	567	2
IconButton	mount	1817	1783	5000
Label	mount	753	749	5000
Layer	mount	4204	4178	5000
Link	mount	854	865	5000
MenuButton	mount	1647	1640	5000
MessageBar	mount	2328	2316	5000
Nav	mount	3057	3082	1000
OverflowSet	mount	1409	1404	5000
Panel	mount	2530	2546	1000
Persona	mount	1256	1248	1000
Pivot	mount	1517	1513	1000
PrimaryButton	mount	1500	1503	5000
Rating	mount	6990	6950	5000
SearchBox	mount	1515	1517	5000
Shimmer	mount	2887	2923	5000
Slider	mount	2091	2134	5000
SpinButton	mount	4251	4215	5000
Spinner	mount	840	836	5000
SplitButton	mount	2873	2838	5000
Stack	mount	862	877	5000
StackWithIntrinsicChildren	mount	2362	2364	5000
StackWithTextChildren	mount	5067	5031	5000
SwatchColorPicker	mount	9542	9532	5000
TagPicker	mount	2310	2366	5000
TeachingBubble	mount	78366	77589	5000
Text	mount	812	817	5000
TextField	mount	1544	1554	5000
ThemeProvider	mount	1444	1456	5000
ThemeProvider	virtual-rerender	1141	1144	5000
ThemeProvider	virtual-rerender-with-unmount	2000	1998	5000
Toggle	mount	1134	1147	5000
buttonNative	mount	545	550	5000

* fix: TableHeaderCell design fixes - Sort arrow is 12px - Sort arrow has a xxs top padding for alignment - Gap between header text and sort icon is xs * changefile * revert unnecessary

fix: TableHeaderCell design fixes

dedcc98

- Sort arrow is 12px - Sort arrow has a xxs top padding for alignment - Gap between header text and sort icon is xs

ling1726 requested a review from a team as a code owner November 18, 2022 13:10

msft-fluent-ui-bot assigned ling1726 Nov 18, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Nov 18, 2022

changefile

788a046

ling1726 mentioned this pull request Nov 18, 2022

[Bug]: Table visual bug bash #25706

Closed

16 tasks

layershifter reviewed Nov 22, 2022

View reviewed changes

layershifter approved these changes Nov 22, 2022

View reviewed changes

merge master

a837879

ling1726 requested review from a team as code owners November 22, 2022 13:39

revert unnecessary

3e526f0

ling1726 removed request for a team November 22, 2022 13:58

ling1726 closed this Nov 22, 2022

ling1726 reopened this Nov 22, 2022

ling1726 merged commit dfe4306 into microsoft:master Nov 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: TableHeaderCell design fixes #25712

fix: TableHeaderCell design fixes #25712

ling1726 commented Nov 18, 2022

size-auditor bot commented Nov 18, 2022 •

edited

Loading

fabricteam commented Nov 18, 2022 •

edited

Loading

codesandbox-ci bot commented Nov 18, 2022 •

edited

Loading

fabricteam commented Nov 18, 2022 •

edited

Loading

fabricteam commented Nov 18, 2022 •

edited

Loading

layershifter Nov 22, 2022

ling1726 Nov 22, 2022 •

edited

Loading

fabricteam commented Nov 22, 2022

fabricteam commented Nov 22, 2022

		ascending: <ArrowUpRegular fontSize={12} />,
		descending: <ArrowDownRegular fontSize={12} />,

fix: TableHeaderCell design fixes #25712

fix: TableHeaderCell design fixes #25712

Conversation

ling1726 commented Nov 18, 2022

size-auditor bot commented Nov 18, 2022 • edited Loading

Asset size changes

fabricteam commented Nov 18, 2022 • edited Loading

📊 Bundle size report

codesandbox-ci bot commented Nov 18, 2022 • edited Loading

fabricteam commented Nov 18, 2022 • edited Loading

Perf Analysis (@fluentui/react-components)

fabricteam commented Nov 18, 2022 • edited Loading

🕵 fluentuiv9 Open the Visual Regressions report to inspect the 20 screenshots

layershifter Nov 22, 2022

Choose a reason for hiding this comment

Before

After

ling1726 Nov 22, 2022 • edited Loading

Choose a reason for hiding this comment

fabricteam commented Nov 22, 2022

🕵 fluentuiv8 Open the Visual Regressions report to inspect the 2 screenshots

fabricteam commented Nov 22, 2022

Perf Analysis (@fluentui/react)

size-auditor bot commented Nov 18, 2022 •

edited

Loading

fabricteam commented Nov 18, 2022 •

edited

Loading

codesandbox-ci bot commented Nov 18, 2022 •

edited

Loading

fabricteam commented Nov 18, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react-components`)

fabricteam commented Nov 18, 2022 •

edited

Loading

ling1726 Nov 22, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react`)