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

BREAKING: TableCell layouts are handled by layout components #24762

Merged
merged 24 commits into from
Sep 13, 2022

Conversation

ling1726
Copy link
Member

@ling1726 ling1726 commented Sep 12, 2022

⚠️ BREAKING CHANGE

Current Behavior

  • TableCell handles the layout of the media slot
  • TablePrimaryCell handles the layout of the media, wrapper, primary and secondary slots
  • Overall Table component uses display: 'flex' to position and align columns.
<TableCell media={<Icon />}>Cell content</TableCell>

<TablePrimaryCell media={<Icon /> primary={"Primary content"} secondary={"Secondary content} />

New Behavior

Uses the table cell layout components implemented in #24773

  • TableCellILayout handles the layout of the media slot
  • TableCellPrimaryLayout handles the layout of the media, wrapper, primary and secondary slots
  • Overall Table component uses native display: 'table' to manage column alignments
<TableCell>
  <TableCellILayoutmedia={<Icon />}>Cell content</TableCellILayout >
</TableCell>

<TableCell>
  <TableCellPrimaryLayoutmedia={<Icon /> primary={"Primary content"} secondary={"Secondary content} />
</TableCell>

Motivation

Flexboxes aren't so easy for tabular data

Flexboxes were a cheap and easy option to do a column layout. However there are issues with it:

  • need to set minWidth to 0 in order to handle long text/content
    • not obvious
  • requires minWidth and maxWidth to explicitly size the column
  • width styles need to be applied to every cell in the column
    • extra effort and can impact performance

These are concerns that are handled for free by native browser display: table. Also the sizing of the columns can be applied only the table header elements and will affect the entire table.

Separate content layout from table layout

Layouting content can require different display properties or extra nested elements that what is required to layout the actual table. Neither v8 DetailsList nor v0 Table actually support layouted cells.

It is preferrable to reduce the complexity of the Table primitives and provide helper layout components to users. This also guarantees flexibility for users who do not want to use the built in layouts.

Related Issue(s)

Fixes #

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
187.939 kB
52.05 kB
react-components
react-components: FluentProvider & webLightTheme
33.359 kB
11.004 kB
react-portal-compat
PortalCompatProvider
5.851 kB
1.964 kB
🤖 This report was generated against 059b48cc66cbeb9c545b60ef842407895cd4e505

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1681 1653 5000
Button mount 1202 1213 5000
FluentProvider mount 2040 1994 5000
FluentProviderWithTheme mount 762 752 10
FluentProviderWithTheme virtual-rerender 703 700 10
FluentProviderWithTheme virtual-rerender-with-unmount 755 739 10
MakeStyles mount 2351 2448 50000
SpinButton mount 3331 3269 5000

@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: 059b48cc66cbeb9c545b60ef842407895cd4e505 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 149 135 1.1:1
ImageMinimalPerf.default 341 320 1.07:1
FlexMinimalPerf.default 258 245 1.05:1
IconMinimalPerf.default 595 568 1.05:1
TreeWith60ListItems.default 132 126 1.05:1
DividerMinimalPerf.default 322 309 1.04:1
PortalMinimalPerf.default 146 140 1.04:1
HeaderMinimalPerf.default 317 307 1.03:1
AttachmentSlotsPerf.default 876 856 1.02:1
ButtonOverridesMissPerf.default 1033 1015 1.02:1
DialogMinimalPerf.default 699 687 1.02:1
DropdownManyItemsPerf.default 551 542 1.02:1
DropdownMinimalPerf.default 2195 2142 1.02:1
FormMinimalPerf.default 338 331 1.02:1
ListWith60ListItems.default 495 486 1.02:1
ProviderMinimalPerf.default 321 316 1.02:1
ReactionMinimalPerf.default 333 327 1.02:1
SkeletonMinimalPerf.default 304 298 1.02:1
TableManyItemsPerf.default 1591 1560 1.02:1
VideoMinimalPerf.default 634 624 1.02:1
AnimationMinimalPerf.default 476 472 1.01:1
AttachmentMinimalPerf.default 126 125 1.01:1
ButtonSlotsPerf.default 423 419 1.01:1
CarouselMinimalPerf.default 357 354 1.01:1
ChatDuplicateMessagesPerf.default 226 224 1.01:1
EmbedMinimalPerf.default 2658 2642 1.01:1
HeaderSlotsPerf.default 682 672 1.01:1
LabelMinimalPerf.default 340 337 1.01:1
RosterPerf.default 1715 1704 1.01:1
ProviderMergeThemesPerf.default 998 990 1.01:1
RadioGroupMinimalPerf.default 391 388 1.01:1
SegmentMinimalPerf.default 310 306 1.01:1
TableMinimalPerf.default 362 357 1.01:1
TextAreaMinimalPerf.default 405 401 1.01:1
AccordionMinimalPerf.default 125 125 1:1
ChatMinimalPerf.default 639 639 1:1
CheckboxMinimalPerf.default 1531 1525 1:1
DatepickerMinimalPerf.default 4670 4671 1:1
InputMinimalPerf.default 859 857 1:1
ItemLayoutMinimalPerf.default 976 977 1:1
LayoutMinimalPerf.default 320 320 1:1
ListCommonPerf.default 508 508 1:1
ListNestedPerf.default 470 468 1:1
LoaderMinimalPerf.default 517 516 1:1
MenuMinimalPerf.default 729 728 1:1
MenuButtonMinimalPerf.default 1366 1371 1:1
SplitButtonMinimalPerf.default 3288 3280 1:1
TextMinimalPerf.default 299 299 1:1
TreeMinimalPerf.default 694 692 1:1
AvatarMinimalPerf.default 160 162 0.99:1
BoxMinimalPerf.default 297 300 0.99:1
CardMinimalPerf.default 460 465 0.99:1
GridMinimalPerf.default 298 301 0.99:1
ListMinimalPerf.default 451 456 0.99:1
RefMinimalPerf.default 184 185 0.99:1
SliderMinimalPerf.default 1216 1224 0.99:1
StatusMinimalPerf.default 603 608 0.99:1
CustomToolbarPrototype.default 2140 2158 0.99:1
ToolbarMinimalPerf.default 795 801 0.99:1
AlertMinimalPerf.default 220 225 0.98:1
PopupMinimalPerf.default 550 559 0.98:1
TooltipMinimalPerf.default 1834 1877 0.98:1
ChatWithPopoverPerf.default 280 297 0.94:1

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 12, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 773 770 5000
Breadcrumb mount 2432 2469 1000
Checkbox mount 2240 2233 5000
CheckboxBase mount 1955 1939 5000
ChoiceGroup mount 3924 3967 5000
ComboBox mount 834 837 1000
CommandBar mount 9142 9496 1000
ContextualMenu mount 10631 10646 1000
DefaultButton mount 969 984 5000
DetailsRow mount 3195 3230 5000
DetailsRowFast mount 3235 3191 5000
DetailsRowNoStyles mount 3060 3090 5000
Dialog mount 2679 2695 1000
DocumentCardTitle mount 153 147 1000
Dropdown mount 2845 2849 5000
FocusTrapZone mount 1624 1616 5000
FocusZone mount 1522 1531 5000
IconButton mount 1479 1495 5000
Label mount 302 307 5000
Layer mount 3909 3796 5000
Link mount 397 400 5000
MenuButton mount 1249 1270 5000
MessageBar mount 1844 1845 5000
Nav mount 2868 2807 1000
OverflowSet mount 929 941 5000
Panel mount 2132 2139 1000
Persona mount 854 865 1000
Pivot mount 1214 1249 1000
PrimaryButton mount 1106 1108 5000
Rating mount 6723 6535 5000
SearchBox mount 1087 1079 5000
Shimmer mount 2418 2464 5000
Slider mount 1668 1689 5000
SpinButton mount 4255 4268 5000
Spinner mount 371 369 5000
SplitButton mount 2638 2666 5000
Stack mount 433 440 5000
StackWithIntrinsicChildren mount 1920 1908 5000
StackWithTextChildren mount 4383 4311 5000
SwatchColorPicker mount 10039 10055 5000
TagPicker mount 2278 2249 5000
TeachingBubble mount 86423 87050 5000
Text mount 369 377 5000
TextField mount 1197 1191 5000
ThemeProvider mount 1106 1115 5000
ThemeProvider virtual-rerender 652 656 5000
ThemeProvider virtual-rerender-with-unmount 1758 1731 5000
Toggle mount 678 696 5000
buttonNative mount 119 112 5000

@ling1726 ling1726 requested a review from a team as a code owner September 12, 2022 19:10
@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 13, 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 c40aae4:

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

ling1726 added a commit to ling1726/fluentui that referenced this pull request Sep 13, 2022
Implements table cell layout components from microsoft#24762 to break up the
aforementioned PR
ling1726 added a commit that referenced this pull request Sep 13, 2022
* feat: Implement table cell layout components

Implements table cell layout components from #24762 to break up the
aforementioned PR

* changefiles
@ling1726 ling1726 marked this pull request as ready for review September 13, 2022 08:14
@ling1726 ling1726 removed the request for review from a team September 13, 2022 08:14
@@ -10,7 +10,16 @@ import {
MoreHorizontalRegular,
} from '@fluentui/react-icons';
import { PresenceBadgeStatus, Avatar, Button } from '@fluentui/react-components';
import { TableBody, TableCell, TableRow, Table, TableHeader, TableHeaderCell, TableCellActions } from '../..';
import {
Copy link
Member Author

Choose a reason for hiding this comment

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

Changes to all these stories are basically:

// Before
<TableCell media={<Icon />}> Content </TableCell>

// After
<TableCell>
  <TableCellLayout media={<Icon />}> Content </TableCellLayout >
</TableCell>

@Hotell Hotell removed their assignment Sep 13, 2022
@ling1726 ling1726 merged commit b51ebae into microsoft:master Sep 13, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Sep 14, 2022
* master: (28 commits)
  Fix value font-weight inside heatmap chart (microsoft#24726)
  Fix legend overflow-indication-text role (microsoft#24756)
  Support custom locale in date axis  (microsoft#24753)
  Cleanup env variables (microsoft#24739)
  ci(github): add GH Action to add issue labels based on new GH issue template (microsoft#24788)
  Update disallowedChangeTypes for newly created packages, to allow only 'prerelease' change types by default (microsoft#24763)
  feat(react-components): Adding missing AvatarGroup exports (microsoft#24770)
  remove unnecessary nohoist (microsoft#24760)
  feat(react-dialog): supports 1st rule of ARIA (microsoft#24525)
  BREAKING: TableCell layouts are handled by layout components (microsoft#24762)
  feat: Implement table cell layout components (microsoft#24773)
  applying package updates
  fix: remove readonly from DetailsList (microsoft#24615)
  chore: Cleaning up tokens in Button components so they better adhere to the design spec (microsoft#24732)
  fix: react-combobox listbox popup width matches trigger width (microsoft#24733)
  fix: react-combobox Option focus outline only shows with keyboard nav (microsoft#24700)
  feat: Publish react-field package, and export from react-components/unstable (microsoft#24235)
  fix: Replacing bottom border styles with text decoration underline in Link (microsoft#24734)
  docs(react-theme): Update readme (microsoft#24755)
  Add tests for hover states (microsoft#24390)
  ...
ling1726 added a commit to ling1726/fluentui that referenced this pull request Sep 22, 2022
Native `display: table` is great for column alignment and simplifies
column sizing. However, native table layout is not very virtualization
friendly and does cannot handle any non-compliant element in the markup.

For virtualization and other cases that involve non-compliant table
elements in markup, there is a `layoutType` prop with the `native`and
`flex` layouts.

Flex layout is what was used before microsoft#24762 with fixes to column
alignment for long content.
ling1726 added a commit that referenced this pull request Sep 29, 2022
* feat: Adds `layoutType` prop to Table with flex option

Native `display: table` is great for column alignment and simplifies
column sizing. However, native table layout is not very virtualization
friendly and does cannot handle any non-compliant element in the markup.

For virtualization and other cases that involve non-compliant table
elements in markup, there is a `layoutType` prop with the `native`and
`flex` layouts.

Flex layout is what was used before #24762 with fixes to column
alignment for long content.

* update test and md

* vr tests should test both layout types

* changefiles

* noNativeElements decides layout

* update changefile
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
)

* feat: Adds `layoutType` prop to Table with flex option

Native `display: table` is great for column alignment and simplifies
column sizing. However, native table layout is not very virtualization
friendly and does cannot handle any non-compliant element in the markup.

For virtualization and other cases that involve non-compliant table
elements in markup, there is a `layoutType` prop with the `native`and
`flex` layouts.

Flex layout is what was used before microsoft#24762 with fixes to column
alignment for long content.

* update test and md

* vr tests should test both layout types

* changefiles

* noNativeElements decides layout

* update changefile
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.

4 participants