Skip to content

Commit

Permalink
Fix propagation of items and groups props from DetailsList through Gr…
Browse files Browse the repository at this point in the history
…oupedList (microsoft#15605)

<!--
!!!!!!! IMPORTANT !!!!!!!

Due to work we're currently doing to prepare master branch for our version 8 beta release,
please hold-off submitting the PR until around October 12 if it's not urgent.
If it is urgent, please submit the PR targeting the 7.0 branch.

This change does not apply to react-northstar contributors.

See microsoft#15222 for more details. Sorry for the inconvenience and short notice.
-->

#### Pull request checklist

- [ ] Addresses an existing issue: Fixes #0000
- [x] Include a change request file using `$ yarn change`

#### Description of changes

_Cherry-pick of microsoft#15321._

_Original PR description:_

Fixed an issue in the recent rework to use `getDerivedStateFromProps` in the `GroupedList` component: the new function was inadvertently checking new input props values *against themselves* rather than the previous state. This issue is now fixed, and the `DetailsList` input state now propagates correctly through `GroupedList`.

Added some unit tests for `DetailsList` explicitly in order to validate that re-rendering a `DetailsList` with new inputs for `items` or `groups` properly updates for each type of transition.

#### Focus areas to test

Validate `DetailsList` when changing the `items` or `groups` props. Added a unit test explicitly for this.
  • Loading branch information
khmakoto authored and Seth Donohue committed Nov 2, 2020
1 parent 43e7a08 commit 7c251f6
Show file tree
Hide file tree
Showing 5 changed files with 1,477 additions and 23 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "minor",
"comment": "Fixing propagation of items and groups props from DetailsList through GroupedList.",
"packageName": "@fluentui/react",
"email": "humbertomakotomorimoto@gmail.com",
"dependentChangeType": "patch",
"date": "2020-10-20T01:11:59.934Z"
}
6 changes: 4 additions & 2 deletions packages/react/etc/react.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1666,12 +1666,14 @@ export interface IGroupedListSectionState {
// @public (undocumented)
export interface IGroupedListState {
// (undocumented)
groups?: IGroup[];
compact?: IGroupedListProps['compact'];
// (undocumented)
lastSelectionMode?: SelectionMode;
groups?: IGroup[];
// (undocumented)
listProps?: IGroupedListProps['listProps'];
// (undocumented)
selectionMode?: IGroupedListProps['selectionMode'];
// (undocumented)
version: {};
}

Expand Down
170 changes: 162 additions & 8 deletions packages/react/src/components/DetailsList/DetailsList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,24 @@ import * as ReactDOM from 'react-dom';
import * as renderer from 'react-test-renderer';
import { ReactWrapper } from 'enzyme';
import { safeMount } from '@uifabric/test-utilities';
import { DetailsList } from './DetailsList';
import { DetailsListBase } from './DetailsList.base';

import { IDetailsList, IColumn, DetailsListLayoutMode, CheckboxVisibility } from './DetailsList.types';
import { IDetailsColumnProps } from './DetailsColumn';
import { IDetailsHeaderProps, DetailsHeader } from './DetailsHeader';
import { EventGroup, IRenderFunction } from '../../Utilities';
import { KeyCodes } from '@uifabric/utilities';
import { IDragDropEvents } from '../../DragDrop';
import { IGroup } from '../../GroupedList';
import { SelectionMode, Selection, SelectionZone } from '../../Selection';
import { getTheme } from '../../Styling';
import { KeyCodes } from '@uifabric/utilities';
import { EventGroup, IRenderFunction } from '../../Utilities';
import { IDetailsColumnProps } from './DetailsColumn';
import { IDetailsHeaderProps, DetailsHeader } from './DetailsHeader';
import { DetailsList } from './DetailsList';
import { DetailsListBase } from './DetailsList.base';
import {
CheckboxVisibility,
DetailsListLayoutMode,
IColumn,
IDetailsGroupDividerProps,
IDetailsList,
} from './DetailsList.types';
import { IDetailsRowProps } from './DetailsRow';

// Populate mock data for testing
function mockData(count: number, isColumn: boolean = false, customDivider: boolean = false): any {
Expand Down Expand Up @@ -484,4 +491,151 @@ describe('DetailsList', () => {
},
);
});

it('handles updates to items and groups', () => {
const tableOneItems = [
{
f1: 'A1',
f2: 'B1',
f3: 'C1',
},
{
f1: 'A2',
f2: 'B2',
f3: 'C2',
},
{
f1: 'A3',
f2: 'B3',
f3: 'C3',
},
{
f1: 'A4',
f2: 'B4',
f3: 'C4',
},
];
const tableTwoItems = [
{
f1: 'D1',
f2: 'E1',
f3: 'F1',
},
{
f1: 'D2',
f2: 'E2',
f3: 'F2',
},
{
f1: 'D3',
f2: 'E3',
f3: 'F3',
},
{
f1: 'D4',
f2: 'E4',
f3: 'F4',
},
];

const groupOneGroups: IGroup[] = [
{ key: 'one-1', name: 'one 1', count: 1, startIndex: 0 },
{ key: 'one-2', name: 'one 2', count: 1, startIndex: 1 },
{ key: 'one-3', name: 'one 3', count: 1, startIndex: 2 },
{ key: 'one-4', name: 'one 4', count: 1, startIndex: 3 },
];

const groupTwoGroups: IGroup[] = [
{ key: 'two-1', name: 'two 1', count: 2, startIndex: 0 },
{ key: 'two-2', name: 'two 2', count: 2, startIndex: 2 },
];

const onRenderDetailsHeader: IRenderFunction<IDetailsHeaderProps> = (headerProps: IDetailsHeaderProps) => {
return (
<div>
{headerProps.columns.map((column: IColumn) => {
return <div key={column.key}>{column.name}</div>;
})}
</div>
);
};

const onRenderRow = (rowProps: IDetailsRowProps) => {
return (
<div>
{rowProps.columns.map((column: IColumn) => {
return <div key={column.key}>{rowProps.item[column.key]}</div>;
})}
</div>
);
};

const onRenderGroupHeader: IRenderFunction<IDetailsGroupDividerProps> = (
groupDividerProps: IDetailsGroupDividerProps,
) => {
return <div>{groupDividerProps.group?.name}</div>;
};

const component = renderer.create(
<DetailsList
onRenderDetailsHeader={onRenderDetailsHeader}
onRenderRow={onRenderRow}
groupProps={{ onRenderHeader: onRenderGroupHeader }}
items={tableOneItems}
groups={groupOneGroups}
layoutMode={DetailsListLayoutMode.fixedColumns}
skipViewportMeasures={true}
/>,
);

expect(component.toJSON()).toMatchSnapshot();

// New items, same groups

component.update(
<DetailsList
onRenderDetailsHeader={onRenderDetailsHeader}
onRenderRow={onRenderRow}
groupProps={{ onRenderHeader: onRenderGroupHeader }}
items={tableTwoItems}
groups={groupOneGroups}
layoutMode={DetailsListLayoutMode.fixedColumns}
skipViewportMeasures={true}
/>,
);

expect(component.toJSON()).toMatchSnapshot();

// Same items, new groups

component.update(
<DetailsList
onRenderDetailsHeader={onRenderDetailsHeader}
onRenderRow={onRenderRow}
groupProps={{ onRenderHeader: onRenderGroupHeader }}
items={tableTwoItems}
groups={groupTwoGroups}
layoutMode={DetailsListLayoutMode.fixedColumns}
skipViewportMeasures={true}
/>,
);

expect(component.toJSON()).toMatchSnapshot();

// New items, same groups

component.update(
<DetailsList
onRenderDetailsHeader={onRenderDetailsHeader}
onRenderRow={onRenderRow}
groupProps={{ onRenderHeader: onRenderGroupHeader }}
items={tableOneItems}
groups={groupTwoGroups}
layoutMode={DetailsListLayoutMode.fixedColumns}
skipViewportMeasures={true}
/>,
);

expect(component.toJSON()).toMatchSnapshot();
});
});
Loading

0 comments on commit 7c251f6

Please sign in to comment.