From d1e3ecb04a94c11bfa987b491001128098713d72 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 25 Jul 2024 12:13:05 -0500 Subject: [PATCH 01/24] Add attribute to make the table height fit to the number of rows --- packages/nimble-components/src/table/index.ts | 3 +++ packages/nimble-components/src/table/styles.ts | 9 ++++++++- packages/nimble-components/src/table/template.ts | 10 +++++----- packages/storybook/src/nimble/table/table.stories.ts | 3 +++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts index 2e480f3fdb..30e8533bb2 100644 --- a/packages/nimble-components/src/table/index.ts +++ b/packages/nimble-components/src/table/index.ts @@ -87,6 +87,9 @@ export class Table< @attr({ attribute: 'selection-mode' }) public selectionMode: TableRowSelectionMode = TableRowSelectionMode.none; + @attr({ attribute: 'fit-height-to-rows', mode: 'boolean' }) + public fitHeightToRows = false; + /** * @internal */ diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 4fc0dec644..88d5ae6e65 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -10,7 +10,8 @@ import { mediumPadding, standardPadding, tableRowBorderColor, - borderHoverColor + borderHoverColor, + controlHeight } from '../theme-provider/design-tokens'; import { Theme } from '../theme-provider/types'; import { hexToRgbaCssColor } from '../utilities/style/colors'; @@ -27,6 +28,12 @@ export const styles = css` height: 480px; --ni-private-column-divider-width: 2px; --ni-private-column-divider-padding: 3px; + + background: lightblue; + } + + :host([fit-height-to-rows]) { + height: min(calc(var(--ni-private-table-scroll-height) + ${controlHeight}), calc(100 * ${controlHeight})); } :host(${focusVisible}) { diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts index 32ae5addd7..a8170798da 100644 --- a/packages/nimble-components/src/table/template.ts +++ b/packages/nimble-components/src/table/template.ts @@ -39,10 +39,8 @@ export const template = html` ${'' /* tabindex managed dynamically by KeyboardNavigationManager */} tabindex="0" aria-multiselectable="${x => x.ariaMultiSelectable}" - ${children({ property: 'childItems', filter: elements() })} - > -
x.scrollX}px; --ni-private-table-header-container-margin-right: ${x => x.virtualizer.headerContainerMarginRight}px; --ni-private-table-scroll-height: ${x => x.virtualizer.scrollHeight}px; @@ -51,7 +49,9 @@ export const template = html
` --ni-private-table-cursor-override: ${x => (x.layoutManager.isColumnBeingSized ? 'col-resize' : 'default')}; --ni-private-table-scrollable-min-width: ${x => x.tableScrollableMinWidth}px; --ni-private-glass-overlay-pointer-events: ${x => (x.layoutManager.isColumnBeingSized ? 'none' : 'default')}; - "> + " + > +
diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 840fa08f41..ca8f907ac2 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -78,6 +78,7 @@ interface TableArgs extends BaseTableArgs { selectionChange: undefined; columnConfigurationChange: undefined; rowExpandToggle: undefined; + fitHeightToRows: boolean; } const simpleData = [ @@ -272,6 +273,7 @@ export const table: StoryObj = { id-field-name="id" data-unused="${x => x.updateData(x)}" parent-id-field-name="parentId" + ?fit-height-to-rows="${x => x.fitHeightToRows}" > <${tableColumnTextTag} column-id="first-name-column" @@ -491,6 +493,7 @@ export const table: StoryObj = { validity: undefined, checkValidity: undefined, tableRef: undefined, + fitHeightToRows: false, updateData: x => { void (async () => { // Safari workaround: the table element instance is made at this point From 2c268400fec941309339d2e43003a470353b3225 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 25 Jul 2024 12:19:27 -0500 Subject: [PATCH 02/24] format --- packages/nimble-components/src/table/styles.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 88d5ae6e65..4073d440a9 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -33,7 +33,10 @@ export const styles = css` } :host([fit-height-to-rows]) { - height: min(calc(var(--ni-private-table-scroll-height) + ${controlHeight}), calc(100 * ${controlHeight})); + height: min( + calc(var(--ni-private-table-scroll-height) + ${controlHeight}), + calc(100 * ${controlHeight}) + ); } :host(${focusVisible}) { From c5f9c51e6e345220f4b3256010aed8d1dc59133c Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 26 Jul 2024 16:06:21 -0500 Subject: [PATCH 03/24] another attempt --- .../nimble-components/src/table/styles.ts | 23 ++++++++++++----- .../nimble-components/src/table/template.ts | 25 ++++++++++--------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 4073d440a9..b463a0c63f 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -26,17 +26,22 @@ export const styles = css` :host { height: 480px; + ${ + /** + * Set a default maximum height for the table of 250 rows (including the header row) so + * that clients don't accidentally create a table that tries to render too many rows at once. + * If needed, the max-height can be overridden by the client, but setting a default ensures + * the max-height is explicitly considered by the client rather than overlooked. + */ '' + } + max-height: calc(250 * ${controlHeight}); + flex-direction: column; --ni-private-column-divider-width: 2px; --ni-private-column-divider-padding: 3px; - - background: lightblue; } :host([fit-height-to-rows]) { - height: min( - calc(var(--ni-private-table-scroll-height) + ${controlHeight}), - calc(100 * ${controlHeight}) - ); + height: auto; } :host(${focusVisible}) { @@ -61,6 +66,12 @@ export const styles = css` font: ${bodyFont}; color: ${bodyFontColor}; cursor: var(--ni-private-table-cursor-override); + flex-grow: 1; + flex-shrink: 1; + } + + :host([fit-height-to-rows]) .table-container { + flex-basis: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); } .glass-overlay { diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts index a8170798da..b2b5af5901 100644 --- a/packages/nimble-components/src/table/template.ts +++ b/packages/nimble-components/src/table/template.ts @@ -39,19 +39,20 @@ export const template = html
` ${'' /* tabindex managed dynamically by KeyboardNavigationManager */} tabindex="0" aria-multiselectable="${x => x.ariaMultiSelectable}" - ${children({ property: 'childItems', filter: elements() })} - style=" - --ni-private-table-scroll-x: -${x => x.scrollX}px; - --ni-private-table-header-container-margin-right: ${x => x.virtualizer.headerContainerMarginRight}px; - --ni-private-table-scroll-height: ${x => x.virtualizer.scrollHeight}px; - --ni-private-table-row-container-top: ${x => x.virtualizer.rowContainerYOffset}px; - --ni-private-table-row-grid-columns: ${x => (x.rowGridColumns ? x.rowGridColumns : '')}; - --ni-private-table-cursor-override: ${x => (x.layoutManager.isColumnBeingSized ? 'col-resize' : 'default')}; - --ni-private-table-scrollable-min-width: ${x => x.tableScrollableMinWidth}px; - --ni-private-glass-overlay-pointer-events: ${x => (x.layoutManager.isColumnBeingSized ? 'none' : 'default')}; - " + ${children({ property: 'childItems', filter: elements() })} > -
+
From 8ad460ef05f34dd08e68c75e8269f7dbff229adb Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 26 Jul 2024 16:07:09 -0500 Subject: [PATCH 04/24] revert indention change --- .../nimble-components/src/table/template.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts index b2b5af5901..32ae5addd7 100644 --- a/packages/nimble-components/src/table/template.ts +++ b/packages/nimble-components/src/table/template.ts @@ -43,16 +43,15 @@ export const template = html
` >
+ --ni-private-table-scroll-x: -${x => x.scrollX}px; + --ni-private-table-header-container-margin-right: ${x => x.virtualizer.headerContainerMarginRight}px; + --ni-private-table-scroll-height: ${x => x.virtualizer.scrollHeight}px; + --ni-private-table-row-container-top: ${x => x.virtualizer.rowContainerYOffset}px; + --ni-private-table-row-grid-columns: ${x => (x.rowGridColumns ? x.rowGridColumns : '')}; + --ni-private-table-cursor-override: ${x => (x.layoutManager.isColumnBeingSized ? 'col-resize' : 'default')}; + --ni-private-table-scrollable-min-width: ${x => x.tableScrollableMinWidth}px; + --ni-private-glass-overlay-pointer-events: ${x => (x.layoutManager.isColumnBeingSized ? 'none' : 'default')}; + ">
From 923dfd571f0861d21288d60b2dfed9713d11d45c Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 26 Jul 2024 16:13:04 -0500 Subject: [PATCH 05/24] rename attr --- packages/nimble-components/src/table/index.ts | 4 ++-- packages/nimble-components/src/table/styles.ts | 4 ++-- packages/storybook/src/nimble/table/table.stories.ts | 11 ++++++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts index 30e8533bb2..32fffdd6ea 100644 --- a/packages/nimble-components/src/table/index.ts +++ b/packages/nimble-components/src/table/index.ts @@ -87,8 +87,8 @@ export class Table< @attr({ attribute: 'selection-mode' }) public selectionMode: TableRowSelectionMode = TableRowSelectionMode.none; - @attr({ attribute: 'fit-height-to-rows', mode: 'boolean' }) - public fitHeightToRows = false; + @attr({ attribute: 'auto-height', mode: 'boolean' }) + public autoHeight = false; /** * @internal diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index b463a0c63f..3c5c377fb8 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -40,7 +40,7 @@ export const styles = css` --ni-private-column-divider-padding: 3px; } - :host([fit-height-to-rows]) { + :host([auto-height]) { height: auto; } @@ -70,7 +70,7 @@ export const styles = css` flex-shrink: 1; } - :host([fit-height-to-rows]) .table-container { + :host([auto-height]) .table-container { flex-basis: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); } diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index ca8f907ac2..850af8c589 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -78,7 +78,7 @@ interface TableArgs extends BaseTableArgs { selectionChange: undefined; columnConfigurationChange: undefined; rowExpandToggle: undefined; - fitHeightToRows: boolean; + autoHeight: boolean; } const simpleData = [ @@ -273,7 +273,7 @@ export const table: StoryObj = { id-field-name="id" data-unused="${x => x.updateData(x)}" parent-id-field-name="parentId" - ?fit-height-to-rows="${x => x.fitHeightToRows}" + ?auto-height="${x => x.autoHeight}" > <${tableColumnTextTag} column-id="first-name-column" @@ -484,6 +484,11 @@ export const table: StoryObj = { 'Event emitted when the user expands or collapses a row in a table with hierarchy. This does not emit when group rows are expanded or collapsed.', control: false, table: { category: apiCategory.events } + }, + autoHeight: { + name: 'auto-height', + description: 'When set to `true`, the table will automatically adjust its height to fit the number of rows in the table.', + table: { category: apiCategory.attributes } } }, args: { @@ -493,7 +498,7 @@ export const table: StoryObj = { validity: undefined, checkValidity: undefined, tableRef: undefined, - fitHeightToRows: false, + autoHeight: false, updateData: x => { void (async () => { // Safari workaround: the table element instance is made at this point From 3d6cf42621c91350772d0713b7c8c76d3f001426 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 26 Jul 2024 16:14:29 -0500 Subject: [PATCH 06/24] Change files --- ...le-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json diff --git a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json new file mode 100644 index 0000000000..f7c62782b2 --- /dev/null +++ b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add attribute to the table to make its height automatically adjust to the number of rows", + "packageName": "@ni/nimble-components", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} From 267bae3b7ed1d50d6d0fe65e644f4b254a92955b Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Fri, 26 Jul 2024 16:23:00 -0500 Subject: [PATCH 07/24] format --- packages/nimble-components/src/table/styles.ts | 4 +++- packages/storybook/src/nimble/table/table.stories.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 3c5c377fb8..f004f04260 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -71,7 +71,9 @@ export const styles = css` } :host([auto-height]) .table-container { - flex-basis: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); + flex-basis: calc( + var(--ni-private-table-scroll-height) + ${controlHeight} + ); } .glass-overlay { diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 850af8c589..ac46dcf338 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -487,7 +487,8 @@ export const table: StoryObj = { }, autoHeight: { name: 'auto-height', - description: 'When set to `true`, the table will automatically adjust its height to fit the number of rows in the table.', + description: + 'When set to `true`, the table will automatically adjust its height to fit the number of rows in the table.', table: { category: apiCategory.attributes } } }, From b4b5e014011da90c75cd22a60b035be4378c0d28 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Mon, 29 Jul 2024 10:07:13 -0500 Subject: [PATCH 08/24] update comment --- packages/nimble-components/src/table/styles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index f004f04260..45f5f6980f 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -31,7 +31,7 @@ export const styles = css` * Set a default maximum height for the table of 250 rows (including the header row) so * that clients don't accidentally create a table that tries to render too many rows at once. * If needed, the max-height can be overridden by the client, but setting a default ensures - * the max-height is explicitly considered by the client rather than overlooked. + * that the max-height is considered if a larger one is needed rather than being overlooked. */ '' } max-height: calc(250 * ${controlHeight}); From f752f369da862791e70ef951cefd985e4e055a64 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Mon, 5 Aug 2024 15:40:44 -0500 Subject: [PATCH 09/24] allow table to be resizable in storybook --- packages/storybook/src/nimble/table/table.stories.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index ac46dcf338..4108895854 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -79,6 +79,7 @@ interface TableArgs extends BaseTableArgs { columnConfigurationChange: undefined; rowExpandToggle: undefined; autoHeight: boolean; + resizable: boolean; } const simpleData = [ @@ -274,6 +275,7 @@ export const table: StoryObj = { data-unused="${x => x.updateData(x)}" parent-id-field-name="parentId" ?auto-height="${x => x.autoHeight}" + style="${x => x.resizable && 'resize: both; overflow: hidden;'}" > <${tableColumnTextTag} column-id="first-name-column" @@ -490,6 +492,12 @@ export const table: StoryObj = { description: 'When set to `true`, the table will automatically adjust its height to fit the number of rows in the table.', table: { category: apiCategory.attributes } + }, + resizable: { + name: 'resizable', + description: + 'Not a property on the table -- temporarily added to demonstrate how the table behaves when `resize: both` is set on the table element.', + table: { category: apiCategory.attributes } } }, args: { @@ -500,6 +508,7 @@ export const table: StoryObj = { checkValidity: undefined, tableRef: undefined, autoHeight: false, + resizable: false, updateData: x => { void (async () => { // Safari workaround: the table element instance is made at this point From 07250935929f2802ea94b7fd79f8458190d78965 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Wed, 14 Aug 2024 15:00:16 -0500 Subject: [PATCH 10/24] token approach --- packages/nimble-components/src/table/index.ts | 3 --- .../nimble-components/src/table/styles.ts | 19 +++++++------------ .../nimble-components/src/table/template.ts | 2 +- .../theme-provider/design-token-comments.ts | 1 + .../src/theme-provider/design-token-names.ts | 1 + .../src/theme-provider/design-tokens.ts | 5 +++++ .../src/nimble/table/table.stories.ts | 7 ++++++- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/nimble-components/src/table/index.ts b/packages/nimble-components/src/table/index.ts index 61f9f5320b..067ec26158 100644 --- a/packages/nimble-components/src/table/index.ts +++ b/packages/nimble-components/src/table/index.ts @@ -87,9 +87,6 @@ export class Table< @attr({ attribute: 'selection-mode' }) public selectionMode: TableRowSelectionMode = TableRowSelectionMode.none; - @attr({ attribute: 'auto-height', mode: 'boolean' }) - public autoHeight = false; - /** * @internal */ diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 45f5f6980f..c103a32d2e 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -11,7 +11,9 @@ import { standardPadding, tableRowBorderColor, borderHoverColor, - controlHeight + controlHeight, + tableFitToRowsHeight, + borderWidth } from '../theme-provider/design-tokens'; import { Theme } from '../theme-provider/types'; import { hexToRgbaCssColor } from '../utilities/style/colors'; @@ -26,22 +28,21 @@ export const styles = css` :host { height: 480px; + ${tableFitToRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + 32px); ${ /** - * Set a default maximum height for the table of 250 rows (including the header row) so + * Set a default maximum height for the table of 40.5 rows plus the header row so * that clients don't accidentally create a table that tries to render too many rows at once. * If needed, the max-height can be overridden by the client, but setting a default ensures * that the max-height is considered if a larger one is needed rather than being overlooked. */ '' } - max-height: calc(250 * ${controlHeight}); + max-height: calc(${controlHeight} + (40.5 * (2 * ${borderWidth} + ${controlHeight}))); flex-direction: column; --ni-private-column-divider-width: 2px; --ni-private-column-divider-padding: 3px; - } - :host([auto-height]) { - height: auto; + background-color: lightblue; } :host(${focusVisible}) { @@ -70,12 +71,6 @@ export const styles = css` flex-shrink: 1; } - :host([auto-height]) .table-container { - flex-basis: calc( - var(--ni-private-table-scroll-height) + ${controlHeight} - ); - } - .glass-overlay { width: 100%; height: 100%; diff --git a/packages/nimble-components/src/table/template.ts b/packages/nimble-components/src/table/template.ts index 32ae5addd7..ef635115fa 100644 --- a/packages/nimble-components/src/table/template.ts +++ b/packages/nimble-components/src/table/template.ts @@ -41,11 +41,11 @@ export const template = html
` aria-multiselectable="${x => x.ariaMultiSelectable}" ${children({ property: 'childItems', filter: elements() })} > +
TableRowSelectionMode[x.selectionMode]}" id-field-name="id" data-unused="${x => x.updateData(x)}" parent-id-field-name="parentId" - ?auto-height="${x => x.autoHeight}" style="${x => x.resizable && 'resize: both; overflow: hidden;'}" > <${tableColumnTextTag} From c75386eb5ade294d8fe6258d0791ffd4e9c04709 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Wed, 14 Aug 2024 15:18:45 -0500 Subject: [PATCH 11/24] updates --- packages/nimble-components/src/table/styles.ts | 7 +------ .../nimble-components/src/theme-provider/design-tokens.ts | 5 +++-- packages/storybook/src/nimble/table/table.stories.ts | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index c103a32d2e..5ef517cec6 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -28,7 +28,7 @@ export const styles = css` :host { height: 480px; - ${tableFitToRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + 32px); + ${tableFitToRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); ${ /** * Set a default maximum height for the table of 40.5 rows plus the header row so @@ -38,11 +38,8 @@ export const styles = css` */ '' } max-height: calc(${controlHeight} + (40.5 * (2 * ${borderWidth} + ${controlHeight}))); - flex-direction: column; --ni-private-column-divider-width: 2px; --ni-private-column-divider-padding: 3px; - - background-color: lightblue; } :host(${focusVisible}) { @@ -67,8 +64,6 @@ export const styles = css` font: ${bodyFont}; color: ${bodyFontColor}; cursor: var(--ni-private-table-cursor-override); - flex-grow: 1; - flex-shrink: 1; } .glass-overlay { diff --git a/packages/nimble-components/src/theme-provider/design-tokens.ts b/packages/nimble-components/src/theme-provider/design-tokens.ts index b581e5d813..f63d15104a 100644 --- a/packages/nimble-components/src/theme-provider/design-tokens.ts +++ b/packages/nimble-components/src/theme-provider/design-tokens.ts @@ -503,10 +503,11 @@ export const spinnerLargeHeight = DesignToken.create( styleNameFromTokenName(tokenNames.spinnerLargeHeight) ).withDefault('64px'); +// The token gets a default value of empty-string but is a value in +// the table styles. export const tableFitToRowsHeight = DesignToken.create( styleNameFromTokenName(tokenNames.tableFitToRowsHeight) -// eslint-disable-next-line no-template-curly-in-string -).withDefault('calc(var(--ni-private-table-scroll-height) + ${controlHeight})'); +).withDefault(''); // Drop Shadow Tokens export const elevation1BoxShadow = DesignToken.create( diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 2b7ce7b36a..cc224212ed 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -498,7 +498,7 @@ export const table: StoryObj = { autoHeight: { name: 'auto-height', description: - 'When set to `true`, the table will automatically adjust its height to fit the number of rows in the table.', + 'TODO', table: { category: apiCategory.attributes } }, resizable: { From 837ee91a3c944b041da0518e23210137ab05b351 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Wed, 14 Aug 2024 15:26:54 -0500 Subject: [PATCH 12/24] format --- packages/storybook/src/nimble/table/table.stories.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index cc224212ed..0b92ee1017 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -1,6 +1,7 @@ import { html, ref } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; +import { tableFitToRowsHeight } from '@ni/nimble-components/src/theme-provider/design-tokens'; import { iconUserTag } from '../../../../nimble-components/src/icons/user'; import { menuTag } from '../../../../nimble-components/src/menu'; import { menuItemTag } from '../../../../nimble-components/src/menu-item'; @@ -24,7 +25,6 @@ import { validityDescription } from '../../utilities/storybook'; import { isChromatic } from '../../utilities/isChromatic'; -import { tableFitToRowsHeight } from '@ni/nimble-components/src/theme-provider/design-tokens'; interface BaseTableArgs extends LabelUserArgs { tableRef: Table; @@ -497,8 +497,7 @@ export const table: StoryObj = { }, autoHeight: { name: 'auto-height', - description: - 'TODO', + description: 'TODO', table: { category: apiCategory.attributes } }, resizable: { From 4a740ceb9e3fe1b35bf68ad691b751cc53667391 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Wed, 14 Aug 2024 15:44:42 -0500 Subject: [PATCH 13/24] fix import --- packages/storybook/src/nimble/table/table.stories.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 0b92ee1017..f2ff37bc25 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -1,7 +1,7 @@ import { html, ref } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; -import { tableFitToRowsHeight } from '@ni/nimble-components/src/theme-provider/design-tokens'; +import { tableFitToRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; import { iconUserTag } from '../../../../nimble-components/src/icons/user'; import { menuTag } from '../../../../nimble-components/src/menu'; import { menuItemTag } from '../../../../nimble-components/src/menu-item'; From ed7a52bfa020ddc900e5373464d9d5fc5263c23d Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 15 Aug 2024 13:17:11 -0500 Subject: [PATCH 14/24] Update docs, matrix story, rename token --- .../nimble-components/src/table/styles.ts | 4 +- .../theme-provider/design-token-comments.ts | 2 +- .../src/theme-provider/design-token-names.ts | 2 +- .../src/theme-provider/design-tokens.ts | 6 +- .../table-fit-rows-height-matrix.stories.ts | 93 +++++++++++++++++++ packages/storybook/src/nimble/table/table.mdx | 7 -- .../src/nimble/table/table.stories.ts | 31 +++---- 7 files changed, 114 insertions(+), 31 deletions(-) create mode 100644 packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts diff --git a/packages/nimble-components/src/table/styles.ts b/packages/nimble-components/src/table/styles.ts index 5ef517cec6..6f8b73e4eb 100644 --- a/packages/nimble-components/src/table/styles.ts +++ b/packages/nimble-components/src/table/styles.ts @@ -12,7 +12,7 @@ import { tableRowBorderColor, borderHoverColor, controlHeight, - tableFitToRowsHeight, + tableFitRowsHeight, borderWidth } from '../theme-provider/design-tokens'; import { Theme } from '../theme-provider/types'; @@ -28,7 +28,7 @@ export const styles = css` :host { height: 480px; - ${tableFitToRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); + ${tableFitRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + ${controlHeight}); ${ /** * Set a default maximum height for the table of 40.5 rows plus the header row so diff --git a/packages/nimble-components/src/theme-provider/design-token-comments.ts b/packages/nimble-components/src/theme-provider/design-token-comments.ts index 3db6cda86a..cd005b83fe 100644 --- a/packages/nimble-components/src/theme-provider/design-token-comments.ts +++ b/packages/nimble-components/src/theme-provider/design-token-comments.ts @@ -74,7 +74,7 @@ export const comments: { readonly [key in TokenName]: string } = { spinnerSmallHeight: 'Small height (16px) for a spinner component', spinnerMediumHeight: 'Medium height (32px) for a spinner component', spinnerLargeHeight: 'Large height (64px) for a spinner component', - tableFitToRowsHeight: 'The height of the table when all rows are visible', + tableFitRowsHeight: 'The height of the table when all rows are visible', smallDelay: 'Elements with small transition areas, such as icons and selection controls, have short durations.', mediumDelay: diff --git a/packages/nimble-components/src/theme-provider/design-token-names.ts b/packages/nimble-components/src/theme-provider/design-token-names.ts index a25ad4f91f..8e1665557e 100644 --- a/packages/nimble-components/src/theme-provider/design-token-names.ts +++ b/packages/nimble-components/src/theme-provider/design-token-names.ts @@ -59,7 +59,7 @@ export const tokenNames: { readonly [key in TokenName]: string } = { spinnerSmallHeight: 'spinner-small-height', spinnerMediumHeight: 'spinner-medium-height', spinnerLargeHeight: 'spinner-large-height', - tableFitToRowsHeight: 'table-fit-to-rows-height', + tableFitRowsHeight: 'table-fit-rows-height', smallDelay: 'small-delay', mediumDelay: 'medium-delay', largeDelay: 'large-delay', diff --git a/packages/nimble-components/src/theme-provider/design-tokens.ts b/packages/nimble-components/src/theme-provider/design-tokens.ts index f63d15104a..0adc325328 100644 --- a/packages/nimble-components/src/theme-provider/design-tokens.ts +++ b/packages/nimble-components/src/theme-provider/design-tokens.ts @@ -505,9 +505,9 @@ export const spinnerLargeHeight = DesignToken.create( // The token gets a default value of empty-string but is a value in // the table styles. -export const tableFitToRowsHeight = DesignToken.create( - styleNameFromTokenName(tokenNames.tableFitToRowsHeight) -).withDefault(''); +export const tableFitRowsHeight = DesignToken.create( + styleNameFromTokenName(tokenNames.tableFitRowsHeight) +).withDefault('480px'); // Drop Shadow Tokens export const elevation1BoxShadow = DesignToken.create( diff --git a/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts new file mode 100644 index 0000000000..9b82662707 --- /dev/null +++ b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts @@ -0,0 +1,93 @@ +import type { Meta, StoryFn } from '@storybook/html'; +import { html, ViewTemplate } from '@microsoft/fast-element'; +import { waitForUpdatesAsync } from '../../../../nimble-components/src/testing/async-helpers'; +import { tableColumnTextTag } from '../../../../nimble-components/src/table-column/text'; +import { Table, tableTag } from '../../../../nimble-components/src/table'; +import type { TableRecord } from '../../../../nimble-components/src/table/types'; +import { tableFitRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; +import { createFixedThemeStory } from '../../utilities/storybook'; +import { createMatrix, sharedMatrixParameters } from '../../utilities/matrix'; +import { backgroundStates } from '../../utilities/states'; + +interface SimpleData extends TableRecord { + firstName: string; + lastName: string; + favoriteColor: string; +} + +const data: SimpleData[] = []; +for (let i = 0; i < 50; i++) { + data.push({ + firstName: `First Name ${i}`, + lastName: `Last Name ${i}`, + favoriteColor: `Favorite Color ${i}` + }); +} + +const groupingStates = [ + ['Not Grouped', undefined], + ['Grouped', 0] +] as const; +type GroupingState = (typeof groupingStates)[number]; + +const metadata: Meta = { + title: 'Tests/Table', + parameters: { + ...sharedMatrixParameters() + } +}; + +export default metadata; + +// prettier-ignore +const component = ( + [_groupingName, groupIndex]: GroupingState +): ViewTemplate => html` + + <${tableTag}> + <${tableColumnTextTag} field-name="firstName" group-index="${() => groupIndex}">First Name + <${tableColumnTextTag} field-name="lastName">Last Name + <${tableColumnTextTag} field-name="favoriteColor">Favorite Color + +`; + +const playFunction = async (rowCount: number): Promise => { + const tableData = data.slice(0, rowCount); + await Promise.all( + Array.from(document.querySelectorAll
(tableTag)).map( + async table => { + await table.setData(tableData); + } + ) + ); +}; + +export const tableFitRowsHeightWith5Rows: StoryFn = createFixedThemeStory( + createMatrix(component, [ + groupingStates + ]), + backgroundStates[0] +); + +tableFitRowsHeightWith5Rows.play = async () => playFunction(5); + +export const tableFitRowsHeightWith10Rows: StoryFn = createFixedThemeStory( + createMatrix(component, [ + groupingStates + ]), + backgroundStates[0] +); +tableFitRowsHeightWith10Rows.play = async () => playFunction(10); + +export const tableFitRowsHeightWith50Rows: StoryFn = createFixedThemeStory( + createMatrix(component, [ + groupingStates + ]), + backgroundStates[0] +); +tableFitRowsHeightWith50Rows.play = async () => playFunction(50); diff --git a/packages/storybook/src/nimble/table/table.mdx b/packages/storybook/src/nimble/table/table.mdx index a51cb7022b..179758cb54 100644 --- a/packages/storybook/src/nimble/table/table.mdx +++ b/packages/storybook/src/nimble/table/table.mdx @@ -21,13 +21,6 @@ and the **Table Column** pages for individual table column types. ## Styling -### Sizing - -The should be sized explicitly or sized to fill the space -of a parent container. The does not currently support -being styled with `height: auto`. The ability to auto size the table is tracked -with [issue 1624](https://github.com/ni/nimble/issues/1624). - ### Full bleed The table will reserve space for expand/collapse row buttons and the collapse diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index f2ff37bc25..d05f3dd3d7 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -1,7 +1,8 @@ import { html, ref } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; -import { tableFitToRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; +import { tableFitRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; +import { scssPropertyFromTokenName } from '../../../../nimble-components/src/theme-provider/design-token-names'; import { iconUserTag } from '../../../../nimble-components/src/icons/user'; import { menuTag } from '../../../../nimble-components/src/menu'; import { menuItemTag } from '../../../../nimble-components/src/menu-item'; @@ -79,8 +80,7 @@ interface TableArgs extends BaseTableArgs { selectionChange: undefined; columnConfigurationChange: undefined; rowExpandToggle: undefined; - autoHeight: boolean; - resizable: boolean; + fitRowsHeight: boolean; } const simpleData = [ @@ -266,12 +266,17 @@ const setSelectedRecordIdsDescription = `A function that makes the rows associat If a record does not exist in the table's data, it will not be selected. If multiple record IDs are specified when the table's selection mode is \`single\`, only the first record that exists in the table's data will become selected.`; +const fitRowsHeightDescription = `Style the table's with \`height: $${tableFitRowsHeight.name};\` to make the table's height grow to fit all rows. + +Note: The table has a default maximum height that will render approximately 40 rows to avoid performance problems. Use caution when overriding the maximum height +of the table because this may lead to performance issues.`; + export const table: StoryObj = { // prettier-ignore render: createUserSelectedThemeStory(html` <${tableTag} @@ -280,7 +285,6 @@ export const table: StoryObj = { id-field-name="id" data-unused="${x => x.updateData(x)}" parent-id-field-name="parentId" - style="${x => x.resizable && 'resize: both; overflow: hidden;'}" > <${tableColumnTextTag} column-id="first-name-column" @@ -495,16 +499,10 @@ export const table: StoryObj = { control: false, table: { category: apiCategory.events } }, - autoHeight: { - name: 'auto-height', - description: 'TODO', - table: { category: apiCategory.attributes } - }, - resizable: { - name: 'resizable', - description: - 'Not a property on the table -- temporarily added to demonstrate how the table behaves when `resize: both` is set on the table element.', - table: { category: apiCategory.attributes } + fitRowsHeight: { + name: 'Fit rows height', + description: fitRowsHeightDescription, + table: { category: apiCategory.styles } } }, args: { @@ -514,8 +512,7 @@ export const table: StoryObj = { validity: undefined, checkValidity: undefined, tableRef: undefined, - autoHeight: false, - resizable: false, + fitRowsHeight: true, updateData: x => { void (async () => { // Safari workaround: the table element instance is made at this point From c280ac09a1658b9f48f76fcbcb203ad4d6c4fb09 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 15 Aug 2024 13:29:29 -0500 Subject: [PATCH 15/24] fixes --- ...ponents-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json | 2 +- .../src/theme-provider/design-tokens.ts | 4 ++-- .../table/table-fit-rows-height-matrix.stories.ts | 12 +++--------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json index f7c62782b2..339d2e9d9f 100644 --- a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json +++ b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json @@ -1,6 +1,6 @@ { "type": "minor", - "comment": "Add attribute to the table to make its height automatically adjust to the number of rows", + "comment": "Add token to allow the table's height to be configured to the height of all table rows", "packageName": "@ni/nimble-components", "email": "20542556+mollykreis@users.noreply.github.com", "dependentChangeType": "patch" diff --git a/packages/nimble-components/src/theme-provider/design-tokens.ts b/packages/nimble-components/src/theme-provider/design-tokens.ts index 0adc325328..60a0c8731a 100644 --- a/packages/nimble-components/src/theme-provider/design-tokens.ts +++ b/packages/nimble-components/src/theme-provider/design-tokens.ts @@ -503,8 +503,8 @@ export const spinnerLargeHeight = DesignToken.create( styleNameFromTokenName(tokenNames.spinnerLargeHeight) ).withDefault('64px'); -// The token gets a default value of empty-string but is a value in -// the table styles. +// The token gets a default value of the table's default height (480px) +// but is given a calculated value in the table styles. export const tableFitRowsHeight = DesignToken.create( styleNameFromTokenName(tokenNames.tableFitRowsHeight) ).withDefault('480px'); diff --git a/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts index 9b82662707..40c1d9177c 100644 --- a/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts +++ b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts @@ -68,26 +68,20 @@ const playFunction = async (rowCount: number): Promise => { }; export const tableFitRowsHeightWith5Rows: StoryFn = createFixedThemeStory( - createMatrix(component, [ - groupingStates - ]), + createMatrix(component, [groupingStates]), backgroundStates[0] ); tableFitRowsHeightWith5Rows.play = async () => playFunction(5); export const tableFitRowsHeightWith10Rows: StoryFn = createFixedThemeStory( - createMatrix(component, [ - groupingStates - ]), + createMatrix(component, [groupingStates]), backgroundStates[0] ); tableFitRowsHeightWith10Rows.play = async () => playFunction(10); export const tableFitRowsHeightWith50Rows: StoryFn = createFixedThemeStory( - createMatrix(component, [ - groupingStates - ]), + createMatrix(component, [groupingStates]), backgroundStates[0] ); tableFitRowsHeightWith50Rows.play = async () => playFunction(50); From 4aec2250a886cfa33d8b1f5381da4ff365b6ecbe Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Thu, 15 Aug 2024 15:00:20 -0500 Subject: [PATCH 16/24] lint --- .../src/nimble/table/table-fit-rows-height-matrix.stories.ts | 1 - packages/storybook/src/nimble/table/table.stories.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts index 40c1d9177c..c6f5402650 100644 --- a/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts +++ b/packages/storybook/src/nimble/table/table-fit-rows-height-matrix.stories.ts @@ -1,6 +1,5 @@ import type { Meta, StoryFn } from '@storybook/html'; import { html, ViewTemplate } from '@microsoft/fast-element'; -import { waitForUpdatesAsync } from '../../../../nimble-components/src/testing/async-helpers'; import { tableColumnTextTag } from '../../../../nimble-components/src/table-column/text'; import { Table, tableTag } from '../../../../nimble-components/src/table'; import type { TableRecord } from '../../../../nimble-components/src/table/types'; diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index d05f3dd3d7..1267b1fb4f 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -2,7 +2,6 @@ import { html, ref } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; import { tableFitRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; -import { scssPropertyFromTokenName } from '../../../../nimble-components/src/theme-provider/design-token-names'; import { iconUserTag } from '../../../../nimble-components/src/icons/user'; import { menuTag } from '../../../../nimble-components/src/menu'; import { menuItemTag } from '../../../../nimble-components/src/menu-item'; From 655811cb5223e46f5243325a59e07742e1b4b9b3 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Mon, 19 Aug 2024 10:38:36 -0500 Subject: [PATCH 17/24] updates --- .../storybook/src/docs/component-status.stories.ts | 13 +++++++------ .../storybook/src/nimble/icon-base/icons.stories.ts | 10 +++++++--- .../storybook/src/nimble/table/table.stories.ts | 4 ++-- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/storybook/src/docs/component-status.stories.ts b/packages/storybook/src/docs/component-status.stories.ts index 67c4339499..7f3190a3ef 100644 --- a/packages/storybook/src/docs/component-status.stories.ts +++ b/packages/storybook/src/docs/component-status.stories.ts @@ -7,6 +7,7 @@ import { mappingIconTag } from '../../../nimble-components/src/mapping/icon'; import { iconCheckTag } from '../../../nimble-components/src/icons/check'; import { iconTriangleTag } from '../../../nimble-components/src/icons/triangle'; import { iconXmarkTag } from '../../../nimble-components/src/icons/xmark'; +import { tableFitRowsHeight } from '../../../nimble-components/src/theme-provider/design-tokens'; import { ComponentFrameworkStatus } from './types'; import { createUserSelectedThemeStory, @@ -559,11 +560,15 @@ const metadata: Meta = { }, // prettier-ignore render: createUserSelectedThemeStory(html` + <${tableTag} ${ref('tableRef')} data-unused="${x => x.updateData(x)}" - ${/* Make the table big enough to remove vertical scrollbar */ ''} - style="height: calc((34px * var(--data-length)) + 32px);" > <${tableColumnAnchorTag} target="_top" column-id="component-name-column" @@ -650,10 +655,6 @@ const metadata: Meta = { const data = components.filter(component => (x.status === 'future' ? isFuture(component) : !isFuture(component))); - x.tableRef.style.setProperty( - '--data-length', - data.length.toString() - ); await x.tableRef.setData(data); })(); }, diff --git a/packages/storybook/src/nimble/icon-base/icons.stories.ts b/packages/storybook/src/nimble/icon-base/icons.stories.ts index 6809da619f..0d33a8015c 100644 --- a/packages/storybook/src/nimble/icon-base/icons.stories.ts +++ b/packages/storybook/src/nimble/icon-base/icons.stories.ts @@ -12,6 +12,7 @@ import { mappingIconTag } from '../../../../nimble-components/src/mapping/icon'; import { tableColumnTextTag } from '../../../../nimble-components/src/table-column/text'; import { IconSeverity } from '../../../../nimble-components/src/icon-base/types'; import { iconMetadata } from '../../../../nimble-components/src/icon-base/tests/icon-metadata'; +import { tableFitRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; import { apiCategory, createUserSelectedThemeStory, @@ -57,7 +58,6 @@ const updateData = (tableRef: Table): void => { // Safari workaround: the table element instance is made at this point // but doesn't seem to be upgraded to a custom element yet await customElements.whenDefined('nimble-table'); - tableRef.style.setProperty('--data-length', data.length.toString()); await tableRef.setData(data); })(); }; @@ -82,10 +82,14 @@ export const icons: StoryObj = { } }, render: createUserSelectedThemeStory(html` + <${tableTag} ${ref('tableRef')} - ${/* Make the table big enough to remove vertical scrollbar */ ''} - style="height: calc((34px * var(--data-length)) + 32px);" data-unused="${x => updateData(x.tableRef)}" > <${tableColumnMappingTag} field-name="tag" key-type="string" fractional-width="0.2" > diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 1267b1fb4f..1a7c2462d5 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -273,7 +273,7 @@ of the table because this may lead to performance issues.`; export const table: StoryObj = { // prettier-ignore render: createUserSelectedThemeStory(html` - <${tableTag} diff --git a/packages/storybook/src/nimble/icon-base/icons.stories.ts b/packages/storybook/src/nimble/icon-base/icons.stories.ts index 0d33a8015c..3786a2e938 100644 --- a/packages/storybook/src/nimble/icon-base/icons.stories.ts +++ b/packages/storybook/src/nimble/icon-base/icons.stories.ts @@ -85,7 +85,7 @@ export const icons: StoryObj = { <${tableTag} From 47dc49da66c1033eed00919a6a9588d30788b35c Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 20 Aug 2024 13:24:37 -0500 Subject: [PATCH 19/24] PR feedback --- .../src/table/tests/table.spec.ts | 67 ++++++++++++++++++- .../theme-provider/design-token-comments.ts | 2 +- packages/nimble-tokens/CONTRIBUTING.md | 2 +- packages/storybook/src/nimble/table/table.mdx | 8 +++ .../src/nimble/table/table.stories.ts | 6 +- 5 files changed, 79 insertions(+), 6 deletions(-) diff --git a/packages/nimble-components/src/table/tests/table.spec.ts b/packages/nimble-components/src/table/tests/table.spec.ts index 39ae1ab072..c1438a6f91 100644 --- a/packages/nimble-components/src/table/tests/table.spec.ts +++ b/packages/nimble-components/src/table/tests/table.spec.ts @@ -5,7 +5,7 @@ import { TableColumn } from '../../table-column/base'; import { TableColumnText, tableColumnTextTag } from '../../table-column/text'; import { TableColumnTextCellView } from '../../table-column/text/cell-view'; import { waitForUpdatesAsync } from '../../testing/async-helpers'; -import { controlHeight } from '../../theme-provider/design-tokens'; +import { controlHeight, tableFitRowsHeight } from '../../theme-provider/design-tokens'; import { waitForEvent } from '../../utilities/testing/component'; import { type Fixture, @@ -2338,6 +2338,71 @@ describe('Table', () => { } ); }); + + describe('styling height with tableFitRowsHeight', () => { + function getExpectedHeight(rowCount: number): string { + const rowHeight = 34; + const headerHeight = 32; + return `${rowCount * rowHeight + headerHeight}px`; + } + + function getTableHeight(): string { + return getComputedStyle(element).getPropertyValue('height'); + } + + beforeEach(async () => { + element.style.height = `var(${tableFitRowsHeight.cssCustomProperty})`; + await connect(); + await waitForUpdatesAsync(); + }); + + it('has correct height before data is applied', () => { + const tokenValue = getTableHeight(); + const expectedHeight = getExpectedHeight(0); + expect(tokenValue).toBe(expectedHeight); + }); + + it('has correct height when height changes because of setting data', async () => { + await element.setData(simpleTableData); + await waitForUpdatesAsync(); + + const tokenValue = getTableHeight(); + const expectedHeight = getExpectedHeight(simpleTableData.length); + expect(tokenValue).toBe(expectedHeight); + }); + + it('has correct height when height changes because grouping is updated', async () => { + await element.setData(simpleTableData); + await waitForUpdatesAsync(); + column1.groupIndex = 0; + await waitForUpdatesAsync(); + + const tokenValue = getTableHeight(); + const expectedHeight = getExpectedHeight(simpleTableData.length * 2); // Each record is grouped into its own group + expect(tokenValue).toBe(expectedHeight); + }); + + it('has correct height when height changes because hierarchy is collapsed', async () => { + const hierarchicalData = [ + { id: '0', parentId: undefined, stringData: 'foo' }, + { id: '1', parentId: '0', stringData: 'foo' }, + { id: '2', parentId: '0', stringData: 'foo' }, + { id: '3', parentId: undefined, stringData: 'foo' } + ]; + + element.idFieldName = 'id'; + element.parentIdFieldName = 'parentId'; + await element.setData(hierarchicalData); + await waitForUpdatesAsync(); + + pageObject.clickCollapseAllButton(); + await waitForUpdatesAsync(); + + const tokenValue = getTableHeight(); + const expectedHeight = getExpectedHeight(2); // There are only 2 top-level parents + expect(tokenValue).toBe(expectedHeight); + }); + }); }); describe('without connection', () => { diff --git a/packages/nimble-components/src/theme-provider/design-token-comments.ts b/packages/nimble-components/src/theme-provider/design-token-comments.ts index cd005b83fe..e7a47b511c 100644 --- a/packages/nimble-components/src/theme-provider/design-token-comments.ts +++ b/packages/nimble-components/src/theme-provider/design-token-comments.ts @@ -74,7 +74,7 @@ export const comments: { readonly [key in TokenName]: string } = { spinnerSmallHeight: 'Small height (16px) for a spinner component', spinnerMediumHeight: 'Medium height (32px) for a spinner component', spinnerLargeHeight: 'Large height (64px) for a spinner component', - tableFitRowsHeight: 'The height of the table when all rows are visible', + tableFitRowsHeight: 'The height of the table when all rows are visible. It is set automatically to the correct value within the scope of each table.', smallDelay: 'Elements with small transition areas, such as icons and selection controls, have short durations.', mediumDelay: diff --git a/packages/nimble-tokens/CONTRIBUTING.md b/packages/nimble-tokens/CONTRIBUTING.md index e4d148b787..036ca58b47 100644 --- a/packages/nimble-tokens/CONTRIBUTING.md +++ b/packages/nimble-tokens/CONTRIBUTING.md @@ -75,5 +75,5 @@ These steps require access to Adobe Illustrator and Perforce so will typically b 4. Generate and build icon components by running `npm run build -w @ni/nimble-components`. This step will report an error at this point but is necessary to enable the next step. 5. Add metadata for the new icons to `nimble-components/src/icon-base/tests/icon-metadata.ts`. 6. Run `npm run build -w @ni/nimble-components` again. It should now succeed. -7. Preview the built files by running: `npm run storybook`, and review the **Icons** story to confirm that your changes appear correctly. Inspect the icons in each **Severity** and ensure their color changes. Ensure the table is large enough to see all icons without nested scrollbars. +7. Preview the built files by running: `npm run storybook`, and review the **Icons** story to confirm that your changes appear correctly. Inspect the icons in each **Severity** and ensure their color changes. 8. Publish a PR with your changes. If there are any new icons, set `changeType` and `dependentChangeType` to minor in the beachball change file. diff --git a/packages/storybook/src/nimble/table/table.mdx b/packages/storybook/src/nimble/table/table.mdx index 179758cb54..27a2a22066 100644 --- a/packages/storybook/src/nimble/table/table.mdx +++ b/packages/storybook/src/nimble/table/table.mdx @@ -2,6 +2,7 @@ import { Controls, Canvas, Meta, Title } from '@storybook/blocks'; import * as tableStories from './table.stories'; import ComponentApisLink from '../../docs/component-apis-link.mdx'; import { tableTag } from '../../../../nimble-components/src/table'; +import { tableFitRowsHeight } from '../../../../nimble-components/src/theme-provider/design-tokens'; @@ -21,6 +22,13 @@ and the **Table Column** pages for individual table column types. ## Styling +### Sizing + +The table's height can be configured to grow to fit all rows without a scrollbar +by styling the table's height with the <code>{tableFitRowsHeight.name}</code> token. + +The table has a default maximum height that will render approximately 40 rows to avoid performance problems. Use caution when overriding the maximum height of the table because this may lead to performance issues. + ### Full bleed The table will reserve space for expand/collapse row buttons and the collapse diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 1a7c2462d5..9270aac1be 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -14,6 +14,7 @@ import { TableRowSelectionMode } from '../../../../nimble-components/src/table/types'; import { ExampleDataType } from '../../../../nimble-components/src/table/tests/types'; +import { scssPropertySetterMarkdown, tokenNames } from '../../../../nimble-components/src/theme-provider/design-token-names'; import { addLabelUseMetadata, type LabelUserArgs @@ -265,10 +266,9 @@ const setSelectedRecordIdsDescription = `A function that makes the rows associat If a record does not exist in the table's data, it will not be selected. If multiple record IDs are specified when the table's selection mode is \`single\`, only the first record that exists in the table's data will become selected.`; -const fitRowsHeightDescription = `Style the table's with \`height: $${tableFitRowsHeight.name};\` to make the table's height grow to fit all rows. +const fitRowsHeightDescription = `Style the table with ${scssPropertySetterMarkdown(tokenNames.tableFitRowsHeight, 'height')} to make the table's height grow to fit all rows. -Note: The table has a default maximum height that will render approximately 40 rows to avoid performance problems. Use caution when overriding the maximum height -of the table because this may lead to performance issues.`; +See the **Sizing** section for information on sizing the table.`; export const table: StoryObj<TableArgs> = { // prettier-ignore From 5a1808cd12d323e43b708f1a6bc6760420f94d12 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 20 Aug 2024 13:36:16 -0500 Subject: [PATCH 20/24] format --- .../nimble-components/src/table/tests/table.spec.ts | 13 ++++++++++--- .../src/theme-provider/design-token-comments.ts | 3 ++- packages/storybook/src/nimble/table/table.mdx | 7 +++++-- .../storybook/src/nimble/table/table.stories.ts | 5 ++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/packages/nimble-components/src/table/tests/table.spec.ts b/packages/nimble-components/src/table/tests/table.spec.ts index c1438a6f91..2fca8d4997 100644 --- a/packages/nimble-components/src/table/tests/table.spec.ts +++ b/packages/nimble-components/src/table/tests/table.spec.ts @@ -5,7 +5,10 @@ import { TableColumn } from '../../table-column/base'; import { TableColumnText, tableColumnTextTag } from '../../table-column/text'; import { TableColumnTextCellView } from '../../table-column/text/cell-view'; import { waitForUpdatesAsync } from '../../testing/async-helpers'; -import { controlHeight, tableFitRowsHeight } from '../../theme-provider/design-tokens'; +import { + controlHeight, + tableFitRowsHeight +} from '../../theme-provider/design-tokens'; import { waitForEvent } from '../../utilities/testing/component'; import { type Fixture, @@ -2367,7 +2370,9 @@ describe('Table', () => { await waitForUpdatesAsync(); const tokenValue = getTableHeight(); - const expectedHeight = getExpectedHeight(simpleTableData.length); + const expectedHeight = getExpectedHeight( + simpleTableData.length + ); expect(tokenValue).toBe(expectedHeight); }); @@ -2378,7 +2383,9 @@ describe('Table', () => { await waitForUpdatesAsync(); const tokenValue = getTableHeight(); - const expectedHeight = getExpectedHeight(simpleTableData.length * 2); // Each record is grouped into its own group + const expectedHeight = getExpectedHeight( + simpleTableData.length * 2 + ); // Each record is grouped into its own group expect(tokenValue).toBe(expectedHeight); }); diff --git a/packages/nimble-components/src/theme-provider/design-token-comments.ts b/packages/nimble-components/src/theme-provider/design-token-comments.ts index e7a47b511c..11a0e32328 100644 --- a/packages/nimble-components/src/theme-provider/design-token-comments.ts +++ b/packages/nimble-components/src/theme-provider/design-token-comments.ts @@ -74,7 +74,8 @@ export const comments: { readonly [key in TokenName]: string } = { spinnerSmallHeight: 'Small height (16px) for a spinner component', spinnerMediumHeight: 'Medium height (32px) for a spinner component', spinnerLargeHeight: 'Large height (64px) for a spinner component', - tableFitRowsHeight: 'The height of the table when all rows are visible. It is set automatically to the correct value within the scope of each table.', + tableFitRowsHeight: + 'The height of the table when all rows are visible. It is set automatically to the correct value within the scope of each table.', smallDelay: 'Elements with small transition areas, such as icons and selection controls, have short durations.', mediumDelay: diff --git a/packages/storybook/src/nimble/table/table.mdx b/packages/storybook/src/nimble/table/table.mdx index 27a2a22066..2de5143dde 100644 --- a/packages/storybook/src/nimble/table/table.mdx +++ b/packages/storybook/src/nimble/table/table.mdx @@ -25,9 +25,12 @@ and the **Table Column** pages for individual table column types. ### Sizing The table's height can be configured to grow to fit all rows without a scrollbar -by styling the table's height with the <code>{tableFitRowsHeight.name}</code> token. +by styling the table's height with the <code>{tableFitRowsHeight.name}</code> +token. -The table has a default maximum height that will render approximately 40 rows to avoid performance problems. Use caution when overriding the maximum height of the table because this may lead to performance issues. +The table has a default maximum height that will render approximately 40 rows to +avoid performance problems. Use caution when overriding the maximum height of +the table because this may lead to performance issues. ### Full bleed diff --git a/packages/storybook/src/nimble/table/table.stories.ts b/packages/storybook/src/nimble/table/table.stories.ts index 9270aac1be..c6e759796e 100644 --- a/packages/storybook/src/nimble/table/table.stories.ts +++ b/packages/storybook/src/nimble/table/table.stories.ts @@ -14,7 +14,10 @@ import { TableRowSelectionMode } from '../../../../nimble-components/src/table/types'; import { ExampleDataType } from '../../../../nimble-components/src/table/tests/types'; -import { scssPropertySetterMarkdown, tokenNames } from '../../../../nimble-components/src/theme-provider/design-token-names'; +import { + scssPropertySetterMarkdown, + tokenNames +} from '../../../../nimble-components/src/theme-provider/design-token-names'; import { addLabelUseMetadata, type LabelUserArgs From f03a2b1b0f38d99ba81ae91fd526bfbb4e0b1888 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 20 Aug 2024 13:53:02 -0500 Subject: [PATCH 21/24] Change files --- ...nimble-tokens-2857f29e-c895-406b-a302-78a6ca683d9d.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@ni-nimble-tokens-2857f29e-c895-406b-a302-78a6ca683d9d.json diff --git a/change/@ni-nimble-tokens-2857f29e-c895-406b-a302-78a6ca683d9d.json b/change/@ni-nimble-tokens-2857f29e-c895-406b-a302-78a6ca683d9d.json new file mode 100644 index 0000000000..6e88592f4b --- /dev/null +++ b/change/@ni-nimble-tokens-2857f29e-c895-406b-a302-78a6ca683d9d.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Create token representing the table's height with all rows visible", + "packageName": "@ni/nimble-tokens", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} From 1a65984dc7c0ae4bac227a174b12261afca9afd5 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:02:22 -0500 Subject: [PATCH 22/24] Update @ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json --- ...ble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json index 339d2e9d9f..ba0065674a 100644 --- a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json +++ b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json @@ -1,7 +1,7 @@ { - "type": "minor", - "comment": "Add token to allow the table's height to be configured to the height of all table rows", + "type": "breaking", + "comment": "Create token representing the table's height with all rows visible; **Breaking change:** the table now specifies a max-height; if a different max-height is required, it needs to be configured.", "packageName": "@ni/nimble-components", "email": "20542556+mollykreis@users.noreply.github.com", - "dependentChangeType": "patch" + "dependentChangeType": "breaking" } From a42e9651941cd33654cdceab7310bca6dcbfb71f Mon Sep 17 00:00:00 2001 From: Milan Raj <rajsite@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:38:07 -0500 Subject: [PATCH 23/24] Update change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json --- ...-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json index ba0065674a..53f5b959a0 100644 --- a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json +++ b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json @@ -3,5 +3,5 @@ "comment": "Create token representing the table's height with all rows visible; **Breaking change:** the table now specifies a max-height; if a different max-height is required, it needs to be configured.", "packageName": "@ni/nimble-components", "email": "20542556+mollykreis@users.noreply.github.com", - "dependentChangeType": "breaking" + "dependentChangeType": "major" } From ba3b6c389fd63a4191060942c6d53e6dafeef873 Mon Sep 17 00:00:00 2001 From: Milan Raj <rajsite@users.noreply.github.com> Date: Tue, 20 Aug 2024 14:38:12 -0500 Subject: [PATCH 24/24] Update change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json --- ...-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json index 53f5b959a0..cf6adffc95 100644 --- a/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json +++ b/change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json @@ -1,5 +1,5 @@ { - "type": "breaking", + "type": "major", "comment": "Create token representing the table's height with all rows visible; **Breaking change:** the table now specifies a max-height; if a different max-height is required, it needs to be configured.", "packageName": "@ni/nimble-components", "email": "20542556+mollykreis@users.noreply.github.com",