Skip to content

Commit

Permalink
Rich text editor | Reverting back the workaround for the "tabindex" v…
Browse files Browse the repository at this point in the history
…alue update in footer buttons (#1550)

# Pull Request

## 🤨 Rationale

<!---
Provide some background and a description of your work.
What problem does this change solve?

Include links to issues, work items, or other discussions.
-->
We recognized that the workaround done in this PR
#1503 creates an issue in iOS mobile
browsers. In mobile/Tab iOS browsers, when the button is clicked, it
does not automatically focus back on the editor, and the keypad closes.

Bug link: https://dev.azure.com/ni/DevCentral/_workitems/edit/2526025

With `pointer-events: none` to `slot="start"` of toggle button:


https://github.com/ni/nimble/assets/123377523/a0ec33d0-2040-4cc9-ac82-2e8c720f2bba

Without `pointer-events: none` to `slot="start"` of toggle button:


https://github.com/ni/nimble/assets/123377523/5c4b0e95-5ea4-4248-8559-7450bcdc2d04

After aligning with the leads, as a quicker resolution, we thought that
this bug would create more impact for the mobile users than the other
`tabindex` issue.

## 👩‍💻 Implementation

<!---
Describe how the change addresses the problem. Consider factors such as
complexity, alternative solutions, performance impact, etc.

Consider listing files with important changes or comment on them
directly in the pull request.
-->
Reverting all the changes that were implemented here:
#1503

## 🧪 Testing

<!---
Detail the testing done to ensure this submission meets requirements. 

Include automated/manual test additions or modifications, testing done
on a local build, private CI run results, and additional testing not
covered by automatic pull request validation. If any functionality is
not covered by automated testing, provide justification.
-->
Manually tested iOS devices(mobile and tab) in Safari and Chrome
browsers.
1. Opened
[nimble.ni.dev](https://nimble.ni.dev/storybook/?path=/docs/incubating-rich-text-editor--docs)
on an iOS device and clicked the buttons, the editor loses the focus.
2. Opened the current [storybook
publish](https://60e89457a987cf003efc0a5b-kqouhrfikv.chromatic.com/) on
an iOS device and clicked the buttons, the editor got the focus as
expected. (As shared in the above recordings)

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------
  • Loading branch information
vivinkrishna-ni authored Sep 20, 2023
1 parent 29b15a0 commit 36b9611
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 45 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fixes keyboard closing when rich-text editor toolbar buttons are clicked in mobile iOS browsers",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
10 changes: 0 additions & 10 deletions packages/nimble-components/src/rich-text/editor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,16 +251,6 @@ 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;
}
:host([error-visible]) .error-icon {
display: none;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,6 @@ export class RichTextEditorPageObject {
await waitForUpdatesAsync();
}

public async clickFooterIconSlot(button: ToolbarButton): Promise<void> {
const icon = this.getIconSlot(button);
icon!.click();
await waitForUpdatesAsync();
}

public getButtonCheckedState(button: ToolbarButton): boolean {
const toggleButton = this.getFormattingButton(button);
return toggleButton!.checked;
Expand Down Expand Up @@ -273,11 +267,4 @@ export class RichTextEditorPageObject {
private getEditorLastChildElement(): Element {
return getLastChildElement(this.getTiptapEditor() as HTMLElement)!;
}

private getIconSlot(
button: ToolbarButton
): HTMLSpanElement | null | undefined {
const toggleButton = this.getFormattingButton(button);
return toggleButton?.shadowRoot?.querySelector('.start');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -208,28 +208,6 @@ 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[] = [];
Expand Down

0 comments on commit 36b9611

Please sign in to comment.