From 7d7a33e100c526e345e7bdf1bffa5520d7545c97 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 15 Jun 2023 09:48:26 -0500 Subject: [PATCH 1/6] Add tooltip to ellipsized table header --- .../src/table-column/base/index.ts | 17 ++++++++- .../src/table-column/base/template.ts | 18 +++++++-- .../src/table/testing/table.pageobject.ts | 15 ++++++++ .../src/table/tests/table.spec.ts | 37 ++++++++++++++++++- 4 files changed, 82 insertions(+), 5 deletions(-) diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts index a86f3323d3..3e353f6483 100644 --- a/packages/nimble-components/src/table-column/base/index.ts +++ b/packages/nimble-components/src/table-column/base/index.ts @@ -1,4 +1,8 @@ -import { attr, nullableNumberConverter } from '@microsoft/fast-element'; +import { + attr, + nullableNumberConverter, + observable +} from '@microsoft/fast-element'; import { FoundationElement } from '@microsoft/fast-foundation'; import { TableColumnSortDirection } from '../../table/types'; import { @@ -41,6 +45,17 @@ export abstract class TableColumn< @attr({ attribute: 'sorting-disabled', mode: 'boolean' }) public sortingDisabled = false; + /** @internal */ + @observable + public isValidContentAndHasOverflow = false; + + /** @internal */ + public headerSpan!: HTMLSpanElement; + + /** @internal */ + @observable + public slottedHeaderContent: HTMLElement[] = []; + public checkValidity(): boolean { return this.columnInternals.validConfiguration; } diff --git a/packages/nimble-components/src/table-column/base/template.ts b/packages/nimble-components/src/table-column/base/template.ts index d8130d08d3..9e44a08fb9 100644 --- a/packages/nimble-components/src/table-column/base/template.ts +++ b/packages/nimble-components/src/table-column/base/template.ts @@ -1,10 +1,22 @@ -import { html } from '@microsoft/fast-element'; +import { html, ref, slotted } from '@microsoft/fast-element'; import type { TableColumn } from '.'; +// prettier-ignore export const template = html` `; diff --git a/packages/nimble-components/src/table/testing/table.pageobject.ts b/packages/nimble-components/src/table/testing/table.pageobject.ts index 3674958486..9ce019aed3 100644 --- a/packages/nimble-components/src/table/testing/table.pageobject.ts +++ b/packages/nimble-components/src/table/testing/table.pageobject.ts @@ -59,6 +59,21 @@ export class TablePageObject { return headers.item(columnIndex); } + public getHeaderTitle(columnIndex: number): string { + const column = this.tableElement.columns[columnIndex]; + return ( + column?.shadowRoot!.firstElementChild?.getAttribute('title') ?? '' + ); + } + + public dispatchEventToHeader( + columnIndex: number, + event: Event + ): boolean | undefined { + const column = this.tableElement.columns[columnIndex]; + return column?.shadowRoot!.firstElementChild?.dispatchEvent(event); + } + public getHeaderRenderedWidth(columnIndex: number): number { const headers = this.tableElement.shadowRoot!.querySelectorAll( 'nimble-table-header' diff --git a/packages/nimble-components/src/table/tests/table.spec.ts b/packages/nimble-components/src/table/tests/table.spec.ts index 4bd9eb8612..2d45b47a04 100644 --- a/packages/nimble-components/src/table/tests/table.spec.ts +++ b/packages/nimble-components/src/table/tests/table.spec.ts @@ -56,7 +56,7 @@ const largeTableData = Array.from(Array(500), (_, i) => { // prettier-ignore async function setup(): Promise>> { return fixture>( - html` + html` stringData @@ -187,6 +187,41 @@ describe('Table', () => { expect(headerContent?.textContent).toEqual('foo'); }); + it('sets title when header text is ellipsized', async () => { + const headerContents = 'a very long value that should get ellipsized due to not fitting within the default header width'; + await element.setData(simpleTableData); + await connect(); + await waitForUpdatesAsync(); + element.columns[0]!.textContent = headerContents; + pageObject.dispatchEventToHeader(0, new MouseEvent('mouseover')); + await waitForUpdatesAsync(); + expect(pageObject.getHeaderTitle(0)).toBe(headerContents); + }); + + it('does not set title when header text is fully visible', async () => { + const headerContents = 'short value'; + await element.setData(simpleTableData); + await connect(); + await waitForUpdatesAsync(); + element.columns[0]!.textContent = headerContents; + pageObject.dispatchEventToHeader(0, new MouseEvent('mouseover')); + await waitForUpdatesAsync(); + expect(pageObject.getHeaderTitle(0)).toBe(''); + }); + + it('removes title on mouseout of header', async () => { + const headerContents = 'a very long value that should get ellipsized due to not fitting within the default header width'; + await element.setData(simpleTableData); + await connect(); + await waitForUpdatesAsync(); + element.columns[0]!.textContent = headerContents; + pageObject.dispatchEventToHeader(0, new MouseEvent('mouseover')); + await waitForUpdatesAsync(); + pageObject.dispatchEventToHeader(0, new MouseEvent('mouseout')); + await waitForUpdatesAsync(); + expect(pageObject.getHeaderTitle(0)).toBe(''); + }); + it('can set data before the element is connected', async () => { await element.setData(simpleTableData); await connect(); From 22bffa1e9dc4190b6d5fde6a6837932608cf67d5 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 15 Jun 2023 09:48:52 -0500 Subject: [PATCH 2/6] Change files --- ...le-components-d849f476-f620-4021-bf50-d5f1258be733.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@ni-nimble-components-d849f476-f620-4021-bf50-d5f1258be733.json diff --git a/change/@ni-nimble-components-d849f476-f620-4021-bf50-d5f1258be733.json b/change/@ni-nimble-components-d849f476-f620-4021-bf50-d5f1258be733.json new file mode 100644 index 0000000000..251e3bf3a2 --- /dev/null +++ b/change/@ni-nimble-components-d849f476-f620-4021-bf50-d5f1258be733.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add tooltip to ellipsized table header", + "packageName": "@ni/nimble-components", + "email": "7282195+m-akinc@users.noreply.github.com", + "dependentChangeType": "patch" +} From 01f08f462ebaca404cdff84e4a6de2944301a78b Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 15 Jun 2023 10:30:20 -0500 Subject: [PATCH 3/6] Trim text content (Firefox fix) --- packages/nimble-components/src/table-column/base/template.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nimble-components/src/table-column/base/template.ts b/packages/nimble-components/src/table-column/base/template.ts index 9e44a08fb9..739aa058bc 100644 --- a/packages/nimble-components/src/table-column/base/template.ts +++ b/packages/nimble-components/src/table-column/base/template.ts @@ -8,13 +8,13 @@ export const template = html` ${ref('headerSpan')} class="header-content" @mouseover="${x => { - x.isValidContentAndHasOverflow = !!x.slottedHeaderContent.map(element => element.textContent).join(' ') + x.isValidContentAndHasOverflow = !!x.slottedHeaderContent.map(element => element.textContent?.trim()).join(' ') && x.headerSpan.offsetWidth < x.headerSpan.scrollWidth; }}" @mouseout="${x => { x.isValidContentAndHasOverflow = false; }}" - title=${x => (x.isValidContentAndHasOverflow ? x.slottedHeaderContent.map(element => element.textContent).join(' ') : null)} + title=${x => (x.isValidContentAndHasOverflow ? x.slottedHeaderContent.map(element => element.textContent?.trim()).join(' ') : null)} > From 8105fdc61868cb20f102e1af4965c0d709b31c81 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 15 Jun 2023 11:38:17 -0500 Subject: [PATCH 4/6] Feedback --- .../nimble-components/src/table-column/base/index.ts | 12 +++++++++++- .../src/table-column/base/template.ts | 8 ++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts index 3e353f6483..d46c7b5012 100644 --- a/packages/nimble-components/src/table-column/base/index.ts +++ b/packages/nimble-components/src/table-column/base/index.ts @@ -54,7 +54,7 @@ export abstract class TableColumn< /** @internal */ @observable - public slottedHeaderContent: HTMLElement[] = []; + public contentSlot?: HTMLSlotElement; public checkValidity(): boolean { return this.columnInternals.validConfiguration; @@ -64,6 +64,16 @@ export abstract class TableColumn< return {}; } + /** @internal */ + public get headerTextContent(): string { + return ( + this.contentSlot + ?.assignedNodes() + .map(node => node.textContent?.trim()) + .join(' ') ?? '' + ); + } + protected abstract getColumnInternalsOptions(): ColumnInternalsOptions; protected sortDirectionChanged(): void { diff --git a/packages/nimble-components/src/table-column/base/template.ts b/packages/nimble-components/src/table-column/base/template.ts index 739aa058bc..ea4f7c97c6 100644 --- a/packages/nimble-components/src/table-column/base/template.ts +++ b/packages/nimble-components/src/table-column/base/template.ts @@ -1,4 +1,4 @@ -import { html, ref, slotted } from '@microsoft/fast-element'; +import { html, ref } from '@microsoft/fast-element'; import type { TableColumn } from '.'; // prettier-ignore @@ -8,15 +8,15 @@ export const template = html` ${ref('headerSpan')} class="header-content" @mouseover="${x => { - x.isValidContentAndHasOverflow = !!x.slottedHeaderContent.map(element => element.textContent?.trim()).join(' ') + x.isValidContentAndHasOverflow = !!x.headerTextContent && x.headerSpan.offsetWidth < x.headerSpan.scrollWidth; }}" @mouseout="${x => { x.isValidContentAndHasOverflow = false; }}" - title=${x => (x.isValidContentAndHasOverflow ? x.slottedHeaderContent.map(element => element.textContent?.trim()).join(' ') : null)} + title=${x => (x.isValidContentAndHasOverflow ? x.headerTextContent : null)} > - + `; From f1605d78ce291a43dba182abb360e2fe0965936f Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Thu, 15 Jun 2023 12:26:06 -0500 Subject: [PATCH 5/6] Use non-null assertion and remove @observable --- packages/nimble-components/src/table-column/base/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts index d46c7b5012..d2df008fc3 100644 --- a/packages/nimble-components/src/table-column/base/index.ts +++ b/packages/nimble-components/src/table-column/base/index.ts @@ -53,8 +53,7 @@ export abstract class TableColumn< public headerSpan!: HTMLSpanElement; /** @internal */ - @observable - public contentSlot?: HTMLSlotElement; + public contentSlot!: HTMLSlotElement; public checkValidity(): boolean { return this.columnInternals.validConfiguration; From d212dca9e30fe73d44cb9d9e5c3680b56bd67db6 Mon Sep 17 00:00:00 2001 From: Mert Akinc <7282195+m-akinc@users.noreply.github.com> Date: Fri, 16 Jun 2023 13:20:27 -0500 Subject: [PATCH 6/6] Remove null checking --- .../nimble-components/src/table-column/base/index.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/nimble-components/src/table-column/base/index.ts b/packages/nimble-components/src/table-column/base/index.ts index d2df008fc3..89f651f44c 100644 --- a/packages/nimble-components/src/table-column/base/index.ts +++ b/packages/nimble-components/src/table-column/base/index.ts @@ -65,12 +65,10 @@ export abstract class TableColumn< /** @internal */ public get headerTextContent(): string { - return ( - this.contentSlot - ?.assignedNodes() - .map(node => node.textContent?.trim()) - .join(' ') ?? '' - ); + return this.contentSlot + .assignedNodes() + .map(node => node.textContent?.trim()) + .join(' '); } protected abstract getColumnInternalsOptions(): ColumnInternalsOptions;