Skip to content

Commit

Permalink
Rich text editor | Fix nested list highlighting (#1462)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale
Fixes #1434

- This PR fixes the behavior of list buttons in case of having nested
lists. Previously for nested lists, the list buttons used to get
highlighted based all all the tags preceding it.

![image](https://github.com/ni/nimble/assets/123591928/35329d4c-387d-4db1-8765-8d2e4764abe3)
- With this change only the list item based on last tag will get
highlighted

![image](https://github.com/ni/nimble/assets/123591928/9e6c10eb-261c-405a-bdca-7272408cef51)


## 👩‍💻 Implementation

- Used `findParentNode` method from tiptap/core to find the current node
type
- Highlight the list button only based on current node type

## 🧪 Testing

- Added unit test for testing this use case

## ✅ 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.

---------

Signed-off-by: Sai krishnan Perumal <[email protected]>
  • Loading branch information
saikrishnan-ni authored Sep 1, 2023
1 parent c0350d7 commit 903a5d2
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix bug with rich text editor nested list button state",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
20 changes: 16 additions & 4 deletions packages/nimble-components/src/rich-text-editor/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import {
FoundationElement
} from '@microsoft/fast-foundation';
import { keyEnter, keySpace } from '@microsoft/fast-web-utilities';
import { Editor, AnyExtension, Extension } from '@tiptap/core';
import {
Editor,
findParentNode,
isList,
AnyExtension,
Extension
} from '@tiptap/core';
import {
schema,
defaultMarkdownParser,
Expand All @@ -30,6 +36,7 @@ import Text from '@tiptap/extension-text';
import { template } from './template';
import { styles } from './styles';
import type { ToggleButton } from '../toggle-button';
import { TipTapNodeName } from './types';
import type { ErrorPattern } from '../patterns/error/types';

declare global {
Expand Down Expand Up @@ -466,10 +473,15 @@ export class RichTextEditor extends FoundationElement implements ErrorPattern {
}

private updateEditorButtonsState(): void {
const { extensionManager, state } = this.tiptapEditor;
const { extensions } = extensionManager;
const { selection } = state;
const parentList = findParentNode((node: { type: { name: string } }) => isList(node.type.name, extensions))(selection);

this.boldButton.checked = this.tiptapEditor.isActive('bold');
this.italicsButton.checked = this.tiptapEditor.isActive('italic');
this.bulletListButton.checked = this.tiptapEditor.isActive('bulletList');
this.numberedListButton.checked = this.tiptapEditor.isActive('orderedList');
this.bulletListButton.checked = parentList?.node.type.name === TipTapNodeName.bulletList;
this.numberedListButton.checked = parentList?.node.type.name === TipTapNodeName.numberedList;
}

private keyActivatesButton(event: KeyboardEvent): boolean {
Expand Down Expand Up @@ -541,7 +553,7 @@ export class RichTextEditor extends FoundationElement implements ErrorPattern {
extensionName: string
): AnyExtension | undefined {
return this.tiptapEditor.extensionManager.extensions.find(
extension => extension.name === extensionName
(extension: { name: string }) => extension.name === extensionName
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ describe('RichTextEditor', () => {
]);
expect(
pageObject.getButtonCheckedState(ToolbarButton.numberedList)
).toBeTrue();
).toBeFalse();
expect(
pageObject.getButtonCheckedState(ToolbarButton.bulletList)
).toBeTrue();
Expand Down Expand Up @@ -534,7 +534,7 @@ describe('RichTextEditor', () => {
).toBeTrue();
expect(
pageObject.getButtonCheckedState(ToolbarButton.bulletList)
).toBeTrue();
).toBeFalse();
});

it('should have "ul" tag names for bullet lists when clicking "tab" to make it nested and "shift+Tab" to make it usual list', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { ToolbarButton } from '../testing/types';
import type { TipTapNodeName } from '../types';

describe('Editor Toolbar button page object types', () => {
it('ToolbarButton fails compile if assigning arbitrary string values', () => {
Expand All @@ -8,3 +9,11 @@ describe('Editor Toolbar button page object types', () => {
expect(value).toEqual('hello');
});
});

describe('Tiptap node types', () => {
it('TipTapNodeName fails compile if assigning arbitrary string values', () => {
// @ts-expect-error This expect will fail if the enum-like type is missing "as const"
const value: TipTapNodeName = 'hello';
expect(value).toEqual('hello');
});
});
11 changes: 11 additions & 0 deletions packages/nimble-components/src/rich-text-editor/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* TipTap node types.
* @public
*/
export const TipTapNodeName = {
bulletList: 'bulletList',
numberedList: 'orderedList'
} as const;

export type TipTapNodeName =
(typeof TipTapNodeName)[keyof typeof TipTapNodeName];

0 comments on commit 903a5d2

Please sign in to comment.