From eec220adbceee8422311cb8fcdb1c73384a33ef2 Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 15:36:31 +0530 Subject: [PATCH 1/8] Adding pointer-events: none to toggle-buttons in toolbar --- packages/nimble-components/src/rich-text/editor/styles.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/nimble-components/src/rich-text/editor/styles.ts b/packages/nimble-components/src/rich-text/editor/styles.ts index 2af9ce8127..77faec8b5d 100644 --- a/packages/nimble-components/src/rich-text/editor/styles.ts +++ b/packages/nimble-components/src/rich-text/editor/styles.ts @@ -219,6 +219,10 @@ export const styles = css` place-items: center; } + nimble-toggle-button::part(start) { + pointer-events: none; + } + :host([error-visible]) .error-icon { display: none; } From e50de646e2f115cadf85f9436853ee8e3de5062a Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 16:02:45 +0530 Subject: [PATCH 2/8] Change files --- ...le-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json diff --git a/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json new file mode 100644 index 0000000000..c06396c5c7 --- /dev/null +++ b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Adding pointer-events: none to toggle-buttons in toolbar", + "packageName": "@ni/nimble-components", + "email": "123377523+vivinkrishna-ni@users.noreply.github.com", + "dependentChangeType": "patch" +} From ff8f38cbc0248e5cab4fc6a7dddfbbbd6f8271a6 Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 16:04:12 +0530 Subject: [PATCH 3/8] Change file --- ...-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json index c06396c5c7..cdff951277 100644 --- a/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json +++ b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json @@ -1,6 +1,6 @@ { "type": "patch", - "comment": "Adding pointer-events: none to toggle-buttons in toolbar", + "comment": "Fix for tabindex value change when clicked on icon of the buttons in rich text editor toolbar.", "packageName": "@ni/nimble-components", "email": "123377523+vivinkrishna-ni@users.noreply.github.com", "dependentChangeType": "patch" From 5d31d4fe2c315571a4d5dec75d52d5c29e86fb77 Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 17:59:54 +0530 Subject: [PATCH 4/8] Added test case to check if the tab index gets updated on icon click --- .../testing/rich-text-editor.pageobject.ts | 11 ++++++++++ .../editor/tests/rich-text-editor.spec.ts | 22 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts index aa639921b6..ad14bfdda9 100644 --- a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts +++ b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts @@ -78,6 +78,12 @@ export class RichTextEditorPageObject { await waitForUpdatesAsync(); } + public async clickFooterIcon(button: ToolbarButton): Promise { + const icon = this.getIcon(button); + icon!.click(); + await waitForUpdatesAsync(); + } + public getButtonCheckedState(button: ToolbarButton): boolean { const toggleButton = this.getFormattingButton(button); return toggleButton!.checked; @@ -217,4 +223,9 @@ export class RichTextEditorPageObject { ); return buttons[button]; } + + private getIcon(button: ToolbarButton): HTMLSpanElement | null | undefined { + const toggleButton = this.getFormattingButton(button); + return toggleButton?.shadowRoot?.querySelector('.start'); + } } diff --git a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts index f693c0d58b..e36e236b00 100644 --- a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts +++ b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts @@ -208,6 +208,28 @@ describe('RichTextEditor', () => { } }); + describe('clicking icons should update the tab index of the button', () => { + const focused: string[] = []; + const disabled: string[] = []; + + for (const value of formattingButtons) { + const specType = getSpecTypeByNamedList(value, focused, disabled); + specType( + `"${value.name}" button icon click check`, + // eslint-disable-next-line @typescript-eslint/no-loop-func + async () => { + await pageObject.clickFooterIcon( + value.toolbarButtonIndex + ); + + expect( + pageObject.getButtonTabIndex(value.toolbarButtonIndex) + ).toBe(0); + } + ); + } + }); + describe('space key press should update the checked state of the buttons', () => { const focused: string[] = []; const disabled: string[] = []; From e7227ddaa3c2352e53db06360cdabe7caf234c20 Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 18:26:58 +0530 Subject: [PATCH 5/8] Fix lint --- .../src/rich-text/editor/tests/rich-text-editor.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts index e36e236b00..b5eb429357 100644 --- a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts +++ b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts @@ -218,9 +218,7 @@ describe('RichTextEditor', () => { `"${value.name}" button icon click check`, // eslint-disable-next-line @typescript-eslint/no-loop-func async () => { - await pageObject.clickFooterIcon( - value.toolbarButtonIndex - ); + await pageObject.clickFooterIcon(value.toolbarButtonIndex); expect( pageObject.getButtonTabIndex(value.toolbarButtonIndex) From 3f1546306a5cf504440263d52e43ba47e890590c Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 18:36:45 +0530 Subject: [PATCH 6/8] Reverting back the test cases --- .../testing/rich-text-editor.pageobject.ts | 11 ---------- .../editor/tests/rich-text-editor.spec.ts | 20 ------------------- 2 files changed, 31 deletions(-) diff --git a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts index ad14bfdda9..aa639921b6 100644 --- a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts +++ b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts @@ -78,12 +78,6 @@ export class RichTextEditorPageObject { await waitForUpdatesAsync(); } - public async clickFooterIcon(button: ToolbarButton): Promise { - const icon = this.getIcon(button); - icon!.click(); - await waitForUpdatesAsync(); - } - public getButtonCheckedState(button: ToolbarButton): boolean { const toggleButton = this.getFormattingButton(button); return toggleButton!.checked; @@ -223,9 +217,4 @@ export class RichTextEditorPageObject { ); return buttons[button]; } - - private getIcon(button: ToolbarButton): HTMLSpanElement | null | undefined { - const toggleButton = this.getFormattingButton(button); - return toggleButton?.shadowRoot?.querySelector('.start'); - } } diff --git a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts index b5eb429357..f693c0d58b 100644 --- a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts +++ b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts @@ -208,26 +208,6 @@ describe('RichTextEditor', () => { } }); - describe('clicking icons should update the tab index of the button', () => { - const focused: string[] = []; - const disabled: string[] = []; - - for (const value of formattingButtons) { - const specType = getSpecTypeByNamedList(value, focused, disabled); - specType( - `"${value.name}" button icon click check`, - // eslint-disable-next-line @typescript-eslint/no-loop-func - async () => { - await pageObject.clickFooterIcon(value.toolbarButtonIndex); - - expect( - pageObject.getButtonTabIndex(value.toolbarButtonIndex) - ).toBe(0); - } - ); - } - }); - describe('space key press should update the checked state of the buttons', () => { const focused: string[] = []; const disabled: string[] = []; From 0d865d20c6ff8e8ddacb7be20b0f973424aded2e Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Fri, 8 Sep 2023 18:45:17 +0530 Subject: [PATCH 7/8] Updated the test case to check the slot containing icon --- .../testing/rich-text-editor.pageobject.ts | 13 +++++++++++ .../editor/tests/rich-text-editor.spec.ts | 22 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts index aa639921b6..7ba013b6a4 100644 --- a/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts +++ b/packages/nimble-components/src/rich-text/editor/testing/rich-text-editor.pageobject.ts @@ -78,6 +78,12 @@ export class RichTextEditorPageObject { await waitForUpdatesAsync(); } + public async clickFooterIconSlot(button: ToolbarButton): Promise { + const icon = this.getIconSlot(button); + icon!.click(); + await waitForUpdatesAsync(); + } + public getButtonCheckedState(button: ToolbarButton): boolean { const toggleButton = this.getFormattingButton(button); return toggleButton!.checked; @@ -217,4 +223,11 @@ export class RichTextEditorPageObject { ); return buttons[button]; } + + private getIconSlot( + button: ToolbarButton + ): HTMLSpanElement | null | undefined { + const toggleButton = this.getFormattingButton(button); + return toggleButton?.shadowRoot?.querySelector('.start'); + } } diff --git a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts index f693c0d58b..e7018a1a00 100644 --- a/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts +++ b/packages/nimble-components/src/rich-text/editor/tests/rich-text-editor.spec.ts @@ -208,6 +208,28 @@ describe('RichTextEditor', () => { } }); + describe('clicking icon slots should update the tab index of the button', () => { + const focused: string[] = []; + const disabled: string[] = []; + + for (const value of formattingButtons) { + const specType = getSpecTypeByNamedList(value, focused, disabled); + specType( + `"${value.name}" button icon click check`, + // eslint-disable-next-line @typescript-eslint/no-loop-func + async () => { + await pageObject.clickFooterIconSlot( + value.toolbarButtonIndex + ); + + expect( + pageObject.getButtonTabIndex(value.toolbarButtonIndex) + ).toBe(0); + } + ); + } + }); + describe('space key press should update the checked state of the buttons', () => { const focused: string[] = []; const disabled: string[] = []; From 3af1a013ad7c400b2d57d71ee87a21e4050f84ae Mon Sep 17 00:00:00 2001 From: Vivin Krishna <123377523+vivinkrishna-ni@users.noreply.github.com> Date: Mon, 11 Sep 2023 10:14:33 +0530 Subject: [PATCH 8/8] Adding placeholder comment for this styling --- ...e-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json | 2 +- packages/nimble-components/src/rich-text/editor/styles.ts | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json index cdff951277..264967ce9d 100644 --- a/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json +++ b/change/@ni-nimble-components-f4a15d51-6bb8-4ee2-8980-eb1b3325b788.json @@ -1,6 +1,6 @@ { "type": "patch", - "comment": "Fix for tabindex value change when clicked on icon of the buttons in rich text editor toolbar.", + "comment": "Update tabindex when clicking icon of a button in rich text editor toolbar.", "packageName": "@ni/nimble-components", "email": "123377523+vivinkrishna-ni@users.noreply.github.com", "dependentChangeType": "patch" diff --git a/packages/nimble-components/src/rich-text/editor/styles.ts b/packages/nimble-components/src/rich-text/editor/styles.ts index 77faec8b5d..0b890ee0a9 100644 --- a/packages/nimble-components/src/rich-text/editor/styles.ts +++ b/packages/nimble-components/src/rich-text/editor/styles.ts @@ -218,7 +218,13 @@ export const styles = css` gap: ${standardPadding}; place-items: center; } - + ${ + /** + * Restricting the pointer-events for the slot="start" where the icon is rendered. + * This can be removed after the below issue is fixed. + * https://github.com/ni/nimble/issues/1422 + */ '' + } nimble-toggle-button::part(start) { pointer-events: none; }