Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add token representing the height of the table with all rows visible #2307

Merged
merged 32 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d1e3ecb
Add attribute to make the table height fit to the number of rows
mollykreis Jul 25, 2024
2c26840
format
mollykreis Jul 25, 2024
c5f9c51
another attempt
mollykreis Jul 26, 2024
8ad460e
revert indention change
mollykreis Jul 26, 2024
923dfd5
rename attr
mollykreis Jul 26, 2024
3d6cf42
Change files
mollykreis Jul 26, 2024
267bae3
format
mollykreis Jul 26, 2024
b4b5e01
update comment
mollykreis Jul 29, 2024
379a784
Merge branch 'main' into table-height-styling
mollykreis Jul 29, 2024
989fe87
Merge branch 'main' into table-height-styling
mollykreis Aug 2, 2024
f752f36
allow table to be resizable in storybook
mollykreis Aug 5, 2024
4f96f3f
Merge branch 'main' into table-height-styling
mollykreis Aug 5, 2024
2984d39
Merge branch 'main' into table-height-styling
mollykreis Aug 14, 2024
0725093
token approach
mollykreis Aug 14, 2024
c75386e
updates
mollykreis Aug 14, 2024
837ee91
format
mollykreis Aug 14, 2024
4a740ce
fix import
mollykreis Aug 14, 2024
5e9ebe2
Merge branch 'main' into table-height-styling
mollykreis Aug 15, 2024
ed7a52b
Update docs, matrix story, rename token
mollykreis Aug 15, 2024
c280ac0
fixes
mollykreis Aug 15, 2024
4aec225
lint
mollykreis Aug 15, 2024
cdb2412
Merge branch 'main' into table-height-styling
mollykreis Aug 19, 2024
655811c
updates
mollykreis Aug 19, 2024
542bca2
unset -> none
mollykreis Aug 19, 2024
7d79ab4
Merge branch 'main' into table-height-styling
mollykreis Aug 20, 2024
47dc49d
PR feedback
mollykreis Aug 20, 2024
5a1808c
format
mollykreis Aug 20, 2024
f03a2b1
Change files
mollykreis Aug 20, 2024
1a65984
Update @ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df26c.json
mollykreis Aug 20, 2024
a42e965
Update change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df…
rajsite Aug 20, 2024
ba3b6c3
Update change/@ni-nimble-components-a5fd15e6-373b-4e3c-b562-fc2e440df…
rajsite Aug 20, 2024
ea03b29
Merge branch 'main' into table-height-styling
rajsite Aug 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
"type": "breaking",
rajsite marked this conversation as resolved.
Show resolved Hide resolved
"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.",
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "breaking"
rajsite marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Create token representing the table's height with all rows visible",
"packageName": "@ni/nimble-tokens",
"email": "[email protected]",
"dependentChangeType": "patch"
}
15 changes: 14 additions & 1 deletion packages/nimble-components/src/table/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import {
mediumPadding,
standardPadding,
tableRowBorderColor,
borderHoverColor
borderHoverColor,
controlHeight,
tableFitRowsHeight,
borderWidth
} from '../theme-provider/design-tokens';
import { Theme } from '../theme-provider/types';
import { hexToRgbaCssColor } from '../utilities/style/colors';
Expand All @@ -25,6 +28,16 @@ export const styles = css`
:host {
height: 480px;
${tableFitRowsHeight.cssCustomProperty}: calc(var(--ni-private-table-scroll-height) + ${controlHeight});
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
${
/**
* 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(${controlHeight} + (40.5 * (2 * ${borderWidth} + ${controlHeight})));
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
--ni-private-column-divider-width: 2px;
--ni-private-column-divider-padding: 3px;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/table/template.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ export const template = html<Table>`
aria-multiselectable="${x => x.ariaMultiSelectable}"
${children({ property: 'childItems', filter: elements() })}
>
<style>:host{ --ni-private-table-scroll-height: ${x => x.virtualizer.scrollHeight}px; }</style>
<div class="table-container ${x => (x.windowShiftKeyDown ? 'disable-select' : '')}"
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')};
Expand Down
74 changes: 73 additions & 1 deletion packages/nimble-components/src/table/tests/table.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } from '../../theme-provider/design-tokens';
import {
controlHeight,
tableFitRowsHeight
} from '../../theme-provider/design-tokens';
import { waitForEvent } from '../../utilities/testing/component';
import {
type Fixture,
Expand Down Expand Up @@ -2338,6 +2341,75 @@ 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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +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.',
smallDelay:
'Elements with small transition areas, such as icons and selection controls, have short durations.',
mediumDelay:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export const tokenNames: { readonly [key in TokenName]: string } = {
spinnerSmallHeight: 'spinner-small-height',
spinnerMediumHeight: 'spinner-medium-height',
spinnerLargeHeight: 'spinner-large-height',
tableFitRowsHeight: 'table-fit-rows-height',
smallDelay: 'small-delay',
mediumDelay: 'medium-delay',
largeDelay: 'large-delay',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,12 @@ export const spinnerLargeHeight = DesignToken.create<string>(
styleNameFromTokenName(tokenNames.spinnerLargeHeight)
).withDefault('64px');

// 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<string>(
styleNameFromTokenName(tokenNames.tableFitRowsHeight)
).withDefault('480px');

// Drop Shadow Tokens
export const elevation1BoxShadow = DesignToken.create<string>(
styleNameFromTokenName(tokenNames.elevation1BoxShadow)
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-tokens/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
13 changes: 7 additions & 6 deletions packages/storybook/src/docs/component-status.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -559,11 +560,15 @@ const metadata: Meta<TableArgs> = {
},
// prettier-ignore
render: createUserSelectedThemeStory(html<TableArgs>`
<style class="code-hide">
${tableTag} {
height: var(${tableFitRowsHeight.cssCustomProperty});
max-height: none;
}
</style>
<${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"
Expand Down Expand Up @@ -650,10 +655,6 @@ const metadata: Meta<TableArgs> = {
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);
})();
},
Expand Down
10 changes: 7 additions & 3 deletions packages/storybook/src/nimble/icon-base/icons.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -57,7 +58,6 @@ const updateData = (tableRef: Table<Data>): 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);
})();
};
Expand All @@ -82,10 +82,14 @@ export const icons: StoryObj<IconArgs> = {
}
},
render: createUserSelectedThemeStory(html`
mollykreis marked this conversation as resolved.
Show resolved Hide resolved
<style class="code-hide">
${tableTag} {
height: var(${tableFitRowsHeight.cssCustomProperty});
max-height: none;
}
</style>
<${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" >
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import type { Meta, StoryFn } from '@storybook/html';
import { html, ViewTemplate } from '@microsoft/fast-element';
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`
<style>
${tableTag} {
height: var(${tableFitRowsHeight.cssCustomProperty});
margin-bottom: 20px;
}
</style>
<${tableTag}>
<${tableColumnTextTag} field-name="firstName" group-index="${() => groupIndex}">First Name</${tableColumnTextTag}>
<${tableColumnTextTag} field-name="lastName">Last Name</${tableColumnTextTag}>
<${tableColumnTextTag} field-name="favoriteColor">Favorite Color</${tableColumnTextTag}>
</${tableTag}>
`;

const playFunction = async (rowCount: number): Promise<void> => {
const tableData = data.slice(0, rowCount);
await Promise.all(
Array.from(document.querySelectorAll<Table>(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);
12 changes: 8 additions & 4 deletions packages/storybook/src/nimble/table/table.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

<Meta of={tableStories} />
<Title of={tableStories} />
Expand All @@ -23,10 +24,13 @@ and the **Table Column** pages for individual table column types.

### Sizing

The <Tag name={tableTag}/> should be sized explicitly or sized to fill the space
of a parent container. The <Tag name={tableTag}/> 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).
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

Expand Down
Loading