From a1e29bd11c83ff16390ef79f14096e172bf53dad Mon Sep 17 00:00:00 2001 From: Alex Okonechnikov Date: Sun, 24 Jul 2022 09:25:32 -0400 Subject: [PATCH 1/4] Make getKey and selection props mutually exclusive --- ...uentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json | 7 +++++++ .../react/src/components/DetailsList/DetailsList.types.ts | 8 +++++++- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json diff --git a/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json b/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json new file mode 100644 index 0000000000000..94a87b8ee1c5e --- /dev/null +++ b/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "DetailsList has been modified to no longer allow \"getKey\" and \"selection\" props to be passed simultaneously. The \"getKey\" property is never used if a Selection object is passed. Either the \"getKey\" prop from the Selection object is used, or Selection's default getKey implementation (item) => item.key is used, but never the \"getKey\" prop passed to DetailsList. Making \"getKey\" and \"selection\" mutually exclusive DetailsList properties prevents users from making this mistake.", + "packageName": "@fluentui/react", + "email": "alexok@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react/src/components/DetailsList/DetailsList.types.ts b/packages/react/src/components/DetailsList/DetailsList.types.ts index a65d835066a73..07c72b42dd31a 100644 --- a/packages/react/src/components/DetailsList/DetailsList.types.ts +++ b/packages/react/src/components/DetailsList/DetailsList.types.ts @@ -66,10 +66,14 @@ export interface IDetailsList extends IList { updateColumn: (column: IColumn, options: { width?: number; newColumnIndex?: number }) => void; } +type ConditionalDetailsListProps = + | { selection?: IDetailsListPropsBase['selection']; getKey?: never } + | { selection?: never; getKey?: IDetailsListPropsBase['getKey'] }; + /** * {@docCategory DetailsList} */ -export interface IDetailsListProps extends IBaseProps, IWithViewportProps { +interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { /** Theme provided by a higher-order component. */ theme?: ITheme; @@ -358,6 +362,8 @@ export interface IDetailsListProps extends IBaseProps, IWithViewpo focusZoneProps?: IFocusZoneProps; } +export type IDetailsListProps = IDetailsListPropsBase & ConditionalDetailsListProps; + /** * {@docCategory DetailsList} */ From 22c8363046144897688e2652918f1008b01c7363 Mon Sep 17 00:00:00 2001 From: Alex Okonechnikov Date: Wed, 27 Jul 2022 01:35:14 +0000 Subject: [PATCH 2/4] fix shimmereddetailslist types + fix build issue --- .../react/src/components/DetailsList/DetailsList.types.ts | 4 ++-- .../react/src/components/DetailsList/DetailsRow.test.tsx | 4 ++-- .../components/DetailsList/ShimmeredDetailsList.types.ts | 6 ++++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/react/src/components/DetailsList/DetailsList.types.ts b/packages/react/src/components/DetailsList/DetailsList.types.ts index 07c72b42dd31a..b91d38ccf5ff8 100644 --- a/packages/react/src/components/DetailsList/DetailsList.types.ts +++ b/packages/react/src/components/DetailsList/DetailsList.types.ts @@ -66,14 +66,14 @@ export interface IDetailsList extends IList { updateColumn: (column: IColumn, options: { width?: number; newColumnIndex?: number }) => void; } -type ConditionalDetailsListProps = +export type ConditionalDetailsListProps = | { selection?: IDetailsListPropsBase['selection']; getKey?: never } | { selection?: never; getKey?: IDetailsListPropsBase['getKey'] }; /** * {@docCategory DetailsList} */ -interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { +export interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { /** Theme provided by a higher-order component. */ theme?: ITheme; diff --git a/packages/react/src/components/DetailsList/DetailsRow.test.tsx b/packages/react/src/components/DetailsList/DetailsRow.test.tsx index d060f131da8d7..ab7378a54da58 100644 --- a/packages/react/src/components/DetailsList/DetailsRow.test.tsx +++ b/packages/react/src/components/DetailsList/DetailsRow.test.tsx @@ -6,7 +6,7 @@ import { SelectionMode, Selection } from '../../Selection'; import { DetailsRow } from './DetailsRow'; import { getTheme } from '../../Styling'; import type { IDetailsRowProps } from './DetailsRow.types'; -import type { IDetailsListProps, IColumn } from './DetailsList.types'; +import type { IColumn } from './DetailsList.types'; const _columns: IColumn[] = [ { @@ -43,7 +43,7 @@ function mockItems(count: number): any[] { const renderRow = (row: IDetailsRowProps) =>
{row.item.name}
; -const mockProps: IDetailsListProps = { +const mockProps = { items: mockItems(5), columns: _columns, onRenderRow: renderRow, diff --git a/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts b/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts index f4ef408038b2b..26c5665f3b333 100644 --- a/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts +++ b/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts @@ -1,14 +1,16 @@ import * as React from 'react'; -import type { IDetailsListProps } from './DetailsList.types'; +import type { ConditionalDetailsListProps, IDetailsListProps, IDetailsListPropsBase } from './DetailsList.types'; import type { IDetailsRowProps } from './DetailsRow.types'; import type { IStyle } from '../../Styling'; import type { IStyleFunctionOrObject } from '../../Utilities'; +export type IShimmeredDetailsListProps = IShimmeredDetailsListPropsBase & ConditionalDetailsListProps; + /** * ShimmeredDetailsList props interface * {@docCategory DetailsList} */ -export interface IShimmeredDetailsListProps extends Omit { +interface IShimmeredDetailsListPropsBase extends Omit { /** * DetailsList styles to pass through. */ From b03a22aad3cf04d0a184560297edd30d2838094e Mon Sep 17 00:00:00 2001 From: Alex Okonechnikov Date: Wed, 27 Jul 2022 11:16:10 +0000 Subject: [PATCH 3/4] update api description and fix documented example of detailslist --- .../DetailsList.Documents.Example.tsx | 2 +- packages/react/etc/react.api.md | 30 +++++++++++-------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/react-examples/src/react/DetailsList/DetailsList.Documents.Example.tsx b/packages/react-examples/src/react/DetailsList/DetailsList.Documents.Example.tsx index 5d2c970b3cb68..8f1443609c7f7 100644 --- a/packages/react-examples/src/react/DetailsList/DetailsList.Documents.Example.tsx +++ b/packages/react-examples/src/react/DetailsList/DetailsList.Documents.Example.tsx @@ -167,6 +167,7 @@ export class DetailsListDocumentsExample extends React.Component<{}, IDetailsLis selectionDetails: this._getSelectionDetails(), }); }, + getKey: this._getKey, }); this.state = { @@ -214,7 +215,6 @@ export class DetailsListDocumentsExample extends React.Component<{}, IDetailsLis compact={isCompactMode} columns={columns} selectionMode={SelectionMode.multiple} - getKey={this._getKey} setKey="multiple" layoutMode={DetailsListLayoutMode.justified} isHeaderVisible={true} diff --git a/packages/react/etc/react.api.md b/packages/react/etc/react.api.md index 93faac95f7aea..316193866dcc4 100644 --- a/packages/react/etc/react.api.md +++ b/packages/react/etc/react.api.md @@ -1073,6 +1073,15 @@ export { concatStyleSets } export { concatStyleSetsWithProps } +// @public (undocumented) +export type ConditionalDetailsListProps = { + selection?: IDetailsListPropsBase['selection']; + getKey?: never; +} | { + selection?: never; + getKey?: IDetailsListPropsBase['getKey']; +}; + // @public (undocumented) export enum ConstrainMode { horizontalConstrained = 1, @@ -4571,7 +4580,10 @@ export interface IDetailsListCheckboxProps extends IDetailsCheckboxProps { } // @public (undocumented) -export interface IDetailsListProps extends IBaseProps, IWithViewportProps { +export type IDetailsListProps = IDetailsListPropsBase & ConditionalDetailsListProps; + +// @public (undocumented) +export interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { // @deprecated ariaLabel?: string; ariaLabelForGrid?: string; @@ -8223,18 +8235,10 @@ export interface IShimmerColors { shimmerWave?: string; } -// @public -export interface IShimmeredDetailsListProps extends Omit { - ariaLabelForShimmer?: string; - detailsListStyles?: IDetailsListProps['styles']; - enableShimmer?: boolean; - onRenderCustomPlaceholder?: (rowProps: IDetailsRowProps, index?: number, defaultRender?: (props: IDetailsRowProps) => React_2.ReactNode) => React_2.ReactNode; - removeFadingOverlay?: boolean; - shimmerLines?: number; - // @deprecated - shimmerOverlayStyles?: IStyleFunctionOrObject; - styles?: IStyleFunctionOrObject; -} +// Warning: (ae-forgotten-export) The symbol "IShimmeredDetailsListPropsBase" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +export type IShimmeredDetailsListProps = IShimmeredDetailsListPropsBase & ConditionalDetailsListProps; // @public export type IShimmeredDetailsListStyleProps = Required>; From 6e77323b685c6af3f6c251016442e4d2b4064f7f Mon Sep 17 00:00:00 2001 From: Alex Okonechnikov Date: Thu, 10 Nov 2022 13:38:48 -0500 Subject: [PATCH 4/4] remove types change and just warn users that selection and getKey are mutually exclusive --- ...-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json | 4 +-- packages/react/etc/react.api.md | 30 ++++++++----------- .../DetailsList/DetailsList.base.tsx | 6 ++++ .../DetailsList/DetailsList.types.ts | 8 +---- .../DetailsList/DetailsRow.test.tsx | 4 +-- .../DetailsList/ShimmeredDetailsList.types.ts | 6 ++-- 6 files changed, 26 insertions(+), 32 deletions(-) diff --git a/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json b/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json index 94a87b8ee1c5e..a91ebdd32a648 100644 --- a/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json +++ b/change/@fluentui-react-519e3c9c-d7fd-40f6-b9f7-6a2d2e1af28f.json @@ -1,6 +1,6 @@ { - "type": "minor", - "comment": "DetailsList has been modified to no longer allow \"getKey\" and \"selection\" props to be passed simultaneously. The \"getKey\" property is never used if a Selection object is passed. Either the \"getKey\" prop from the Selection object is used, or Selection's default getKey implementation (item) => item.key is used, but never the \"getKey\" prop passed to DetailsList. Making \"getKey\" and \"selection\" mutually exclusive DetailsList properties prevents users from making this mistake.", + "type": "patch", + "comment": "DetailsList has been modified to warn when \"getKey\" and \"selection\" props are passed simultaneously. The \"getKey\" property is never used if a Selection object is passed. Either the \"getKey\" prop from the Selection object is used, or Selection's default getKey implementation (item) => item.key is used, but never the \"getKey\" prop passed to DetailsList. Making \"getKey\" and \"selection\" mutually exclusive DetailsList properties prevents users from making this mistake.", "packageName": "@fluentui/react", "email": "alexok@microsoft.com", "dependentChangeType": "patch" diff --git a/packages/react/etc/react.api.md b/packages/react/etc/react.api.md index 316193866dcc4..93faac95f7aea 100644 --- a/packages/react/etc/react.api.md +++ b/packages/react/etc/react.api.md @@ -1073,15 +1073,6 @@ export { concatStyleSets } export { concatStyleSetsWithProps } -// @public (undocumented) -export type ConditionalDetailsListProps = { - selection?: IDetailsListPropsBase['selection']; - getKey?: never; -} | { - selection?: never; - getKey?: IDetailsListPropsBase['getKey']; -}; - // @public (undocumented) export enum ConstrainMode { horizontalConstrained = 1, @@ -4580,10 +4571,7 @@ export interface IDetailsListCheckboxProps extends IDetailsCheckboxProps { } // @public (undocumented) -export type IDetailsListProps = IDetailsListPropsBase & ConditionalDetailsListProps; - -// @public (undocumented) -export interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { +export interface IDetailsListProps extends IBaseProps, IWithViewportProps { // @deprecated ariaLabel?: string; ariaLabelForGrid?: string; @@ -8235,10 +8223,18 @@ export interface IShimmerColors { shimmerWave?: string; } -// Warning: (ae-forgotten-export) The symbol "IShimmeredDetailsListPropsBase" needs to be exported by the entry point index.d.ts -// -// @public (undocumented) -export type IShimmeredDetailsListProps = IShimmeredDetailsListPropsBase & ConditionalDetailsListProps; +// @public +export interface IShimmeredDetailsListProps extends Omit { + ariaLabelForShimmer?: string; + detailsListStyles?: IDetailsListProps['styles']; + enableShimmer?: boolean; + onRenderCustomPlaceholder?: (rowProps: IDetailsRowProps, index?: number, defaultRender?: (props: IDetailsRowProps) => React_2.ReactNode) => React_2.ReactNode; + removeFadingOverlay?: boolean; + shimmerLines?: number; + // @deprecated + shimmerOverlayStyles?: IStyleFunctionOrObject; + styles?: IStyleFunctionOrObject; +} // @public export type IShimmeredDetailsListStyleProps = Required>; diff --git a/packages/react/src/components/DetailsList/DetailsList.base.tsx b/packages/react/src/components/DetailsList/DetailsList.base.tsx index 963abbe06ab56..19945fd737d5a 100644 --- a/packages/react/src/components/DetailsList/DetailsList.base.tsx +++ b/packages/react/src/components/DetailsList/DetailsList.base.tsx @@ -9,6 +9,7 @@ import { getRTLSafeKeyCode, classNamesFunction, memoizeFunction, + warnMutuallyExclusive, } from '../../Utilities'; import { CheckboxVisibility, @@ -57,6 +58,7 @@ import type { IGroupedList, IGroupDividerProps, IGroupRenderProps, IGroup } from import type { IListProps } from '../../List'; const getClassNames = classNamesFunction(); +const COMPONENT_NAME = 'DetailsList'; export interface IDetailsListState { focusedItemIndex: number; @@ -801,6 +803,10 @@ export class DetailsListBase extends React.Component void; } -export type ConditionalDetailsListProps = - | { selection?: IDetailsListPropsBase['selection']; getKey?: never } - | { selection?: never; getKey?: IDetailsListPropsBase['getKey'] }; - /** * {@docCategory DetailsList} */ -export interface IDetailsListPropsBase extends IBaseProps, IWithViewportProps { +export interface IDetailsListProps extends IBaseProps, IWithViewportProps { /** Theme provided by a higher-order component. */ theme?: ITheme; @@ -362,8 +358,6 @@ export interface IDetailsListPropsBase extends IBaseProps, IWithVi focusZoneProps?: IFocusZoneProps; } -export type IDetailsListProps = IDetailsListPropsBase & ConditionalDetailsListProps; - /** * {@docCategory DetailsList} */ diff --git a/packages/react/src/components/DetailsList/DetailsRow.test.tsx b/packages/react/src/components/DetailsList/DetailsRow.test.tsx index ab7378a54da58..d060f131da8d7 100644 --- a/packages/react/src/components/DetailsList/DetailsRow.test.tsx +++ b/packages/react/src/components/DetailsList/DetailsRow.test.tsx @@ -6,7 +6,7 @@ import { SelectionMode, Selection } from '../../Selection'; import { DetailsRow } from './DetailsRow'; import { getTheme } from '../../Styling'; import type { IDetailsRowProps } from './DetailsRow.types'; -import type { IColumn } from './DetailsList.types'; +import type { IDetailsListProps, IColumn } from './DetailsList.types'; const _columns: IColumn[] = [ { @@ -43,7 +43,7 @@ function mockItems(count: number): any[] { const renderRow = (row: IDetailsRowProps) =>
{row.item.name}
; -const mockProps = { +const mockProps: IDetailsListProps = { items: mockItems(5), columns: _columns, onRenderRow: renderRow, diff --git a/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts b/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts index 26c5665f3b333..f4ef408038b2b 100644 --- a/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts +++ b/packages/react/src/components/DetailsList/ShimmeredDetailsList.types.ts @@ -1,16 +1,14 @@ import * as React from 'react'; -import type { ConditionalDetailsListProps, IDetailsListProps, IDetailsListPropsBase } from './DetailsList.types'; +import type { IDetailsListProps } from './DetailsList.types'; import type { IDetailsRowProps } from './DetailsRow.types'; import type { IStyle } from '../../Styling'; import type { IStyleFunctionOrObject } from '../../Utilities'; -export type IShimmeredDetailsListProps = IShimmeredDetailsListPropsBase & ConditionalDetailsListProps; - /** * ShimmeredDetailsList props interface * {@docCategory DetailsList} */ -interface IShimmeredDetailsListPropsBase extends Omit { +export interface IShimmeredDetailsListProps extends Omit { /** * DetailsList styles to pass through. */