Cherry pick: GroupedListV2 changes#25985
Merged
spmonahan merged 21 commits intomicrosoft:7.0from Dec 20, 2022
Merged
Conversation
|
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 cbcfafe:
|
Asset size changes
Baseline commit: 8c26b4e2c34095aa7fdbf3dee180b52b553833dd (build) |
Collaborator
🕵 FluentUI-v7 No visual regressions between this PR and main |
…osoft#24460) * GroupedList: add new version to address virtualization issues Introduces a new component, GroupedListV2, that is a drop-in replacement for GroupedList. GroupedListV2 addresses a bug with the virtualization implementation in GroupedList. As it is a significant re-write of the internals we've decided to make it a new component so users can opt in to the new component/behavior as needed rather than risk significant breakage with the existing GroupedList implementation. --- Virtualization in GroupedList is powered by List and groups in GroupedList are nested Lists. When nested with two or more levels of groups issues can arise with virtualization that result in the vertical size of the Lists being miscalculated resulting in items not rendering, the scrollbar repeatedly resizing (causing the list to "jump" about), or both. List does work asynchronously which contributes to the issue itself and makes debugging practically impossible as even a simple GroupedList will contain many Lists all of which are virtualized and rendering async. To address this issue we are introducing GroupedListV2 which is a drop-in replacement for GropedList (V1) as it adheres to the same API. Internally GroupedListV2 flattens virtualization into a single List eliminating the virtualization bug described above and making the list easier to reason about and debug. * List: add conditional rendering option Adds support to conditionally render cells in List which helps when rendering flattend GroupedLists as we don't really know if we need to render certain parts of the list (e.g., footers) until we call the render function. Ensure GroupedList <--> GroupedListV2 compatibility. * DetailsList: allow for custom GroupedList renderer This change allows users to provide a custom GroupedList renderer like GroupedListV2. * Update @fluentui/react API snapshot Add GroupedListV2 tests Add DetailsList tests A dd support for ungrouped lists * add perf tests for groupedlist/groupedlistv2 * change files * better types and refactor render functions. * refactor grouped items * typescript * WIP debugging * fix issues from tests - Add proper `getKey()` handling. - Remove selection dependency for "show all" and footer rendering. * Mark GroupedListV2 as unstable * groupedlistv2: update naming - Rename to GroupedListV2_unstable - Update tests to use this name * update api snapshot * update groupedlistv2 import for perf-test * update snapshots * pr feedback * update test snapshot
…11y experience (microsoft#17591) * GroupedDetailsList will now have correct row-index for GroupHeader and correct offset is passed to DetailsRow which accounts for GroupHeader * DetailsRow now takes groups as a prop which is used to determine aria-posinset for GroupedList * updated GroupedList examples to pass groups prop to DetailsRow * updated react api file * updated detailsrow documentationa and minor fix * updated react snapshots * updated react examples snapshots * Change files * updated change file * minor * add aria-rowindex attribute to GroupedHeader.base * updated react snapshots * comment cleanup * fix: build errors * fix: build error * added missing dependency * fix CI error * getItemGroup now returns the row item's exact group * remove first for-of loop in getItemGroup * fix CI error * update getItemGroup to remove ! operator * update getTotalRowCount to remove ! operator * update DetailsRow props * remove aria-posinset calculation from DetailsRow * ariaPosinSet and ariaSetSize are now passed in return function of onRenderGroupCell for consumers to use * update GroupedList examples topass ariaPosInSet & ariaSetSize to DetailsRow * update api * ci fix * add useGroupedDetailsListIndexMap for O(1) lookup of helper variables to calculate correct index for GroupedDetailsList * GroupedListSection now passes group to onRenderCell, removes ariaPosInSet and ariaSetSize * DetailsRow now accepts group prop and calculates ariaPosInSet and ariaSetSize based on group * update GroupedList examples to pass group prop to DetailsRow * update API * update DetailsRow type documentation and ci fix * ci fix
35d16f0 to
4a11d53
Compare
Contributor
Author
|
I've accepted the Screener change as it was Coachmark which has an animation and is known to have false positives as a result. |
GroupedListV2 depends on some updates to List that were not present in the v7 branch. These List changes broke some DetailsList tests so those have been updated as well.
Collaborator
Perf AnalysisAll results
|
gggjtgxing
approved these changes
Dec 19, 2022
khmakoto
reviewed
Dec 19, 2022
khmakoto
reviewed
Dec 19, 2022
change/@fluentui-react-8cc34e48-8e55-4e4c-9ddb-f1f9cd3b0f3f.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-b8d144d8-e712-440f-b5c7-ab174642cf85.json
Outdated
Show resolved
Hide resolved
khmakoto
reviewed
Dec 20, 2022
khmakoto
reviewed
Dec 20, 2022
Comment on lines
+2
to
+15
| import { | ||
| DetailsHeader, | ||
| DetailsList, | ||
| IColumn, | ||
| IDetailsHeaderProps, | ||
| IDetailsList, | ||
| IGroup, | ||
| IRenderFunction, | ||
| IToggleStyles, | ||
| mergeStyles, | ||
| Toggle, | ||
| GroupedListV2_unstable as GroupedListV2, | ||
| } from 'office-ui-fabric-react'; | ||
| import { DefaultButton, IButtonStyles } from 'office-ui-fabric-react/lib/Button'; |
Member
There was a problem hiding this comment.
nit: You can pull all of these up into one import
Suggested change
| import { | |
| DetailsHeader, | |
| DetailsList, | |
| IColumn, | |
| IDetailsHeaderProps, | |
| IDetailsList, | |
| IGroup, | |
| IRenderFunction, | |
| IToggleStyles, | |
| mergeStyles, | |
| Toggle, | |
| GroupedListV2_unstable as GroupedListV2, | |
| } from 'office-ui-fabric-react'; | |
| import { DefaultButton, IButtonStyles } from 'office-ui-fabric-react/lib/Button'; | |
| import { | |
| DefaultButton, | |
| DetailsHeader, | |
| DetailsList, | |
| IButtonStyles, | |
| IColumn, | |
| IDetailsHeaderProps, | |
| IDetailsList, | |
| IGroup, | |
| IRenderFunction, | |
| IToggleStyles, | |
| mergeStyles, | |
| Toggle, | |
| GroupedListV2_unstable as GroupedListV2, | |
| } from 'office-ui-fabric-react'; |
Comment on lines
+2
to
+9
| import { | ||
| DetailsHeader, | ||
| DetailsList, | ||
| IColumn, | ||
| IDetailsHeaderProps, | ||
| IGroup, | ||
| } from 'office-ui-fabric-react/lib/DetailsList'; | ||
| import { GroupedListV2_unstable as GroupedListV2 } from 'office-ui-fabric-react/lib/GroupedListV2'; |
Member
There was a problem hiding this comment.
nit: You can pull these up into one import statement
Suggested change
| import { | |
| DetailsHeader, | |
| DetailsList, | |
| IColumn, | |
| IDetailsHeaderProps, | |
| IGroup, | |
| } from 'office-ui-fabric-react/lib/DetailsList'; | |
| import { GroupedListV2_unstable as GroupedListV2 } from 'office-ui-fabric-react/lib/GroupedListV2'; | |
| import { | |
| DetailsHeader, | |
| DetailsList, | |
| IColumn, | |
| IDetailsHeaderProps, | |
| IGroup, | |
| GroupedListV2_unstable as GroupedListV2, | |
| } from 'office-ui-fabric-react/lib/DetailsList'; |
khmakoto
reviewed
Dec 20, 2022
packages/office-ui-fabric-react/src/components/GroupedList/GroupedListV2.base.tsx
Show resolved
Hide resolved
khmakoto
approved these changes
Dec 20, 2022
Contributor
Author
|
Admin merging to override the bundle size change |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous Behavior
Fluent UI v7 did not have
GroupedListV2and the associated fixes.New Behavior
GroupedListV2has been cherry-picked into Fluent UI v7.Related Issue(s)