-
Notifications
You must be signed in to change notification settings - Fork 178
Fix #3171 #3184 #3211
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
base: master
Are you sure you want to change the base?
Fix #3171 #3184 #3211
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes two accessibility issues related to Tab key handling: keyboard focus getting trapped in the editor (#3171) and Shift+Tab not working properly with bulleted lists (#3184). The solution introduces granular control over Tab key behaviors through a new HandleTabOptions interface, allowing developers to selectively enable/disable specific Tab features like paragraph indentation, list indentation, and table operations. Additionally, it moves the normalizeContentModel call into setModelIndentation to properly clean up empty list items after outdenting, preventing them from incorrectly triggering tab handlers.
Key changes:
- Introduced
HandleTabOptionsinterface with five boolean properties to control specific Tab key behaviors - Moved
normalizeContentModelcall fromsetIndentationtosetModelIndentationfor consistent list cleanup - Updated
EditPluginto parse boolean or objecthandleTabKeyoptions and convert them toRequired<HandleTabOptions>
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/roosterjs-content-model-plugins/lib/edit/EditOptions.ts | Adds HandleTabOptions interface and updates EditOptions.handleTabKey to accept boolean or HandleTabOptions |
| packages/roosterjs-content-model-plugins/lib/edit/EditPlugin.ts | Updates constructor to parse handleTabKey options and convert to Required object |
| packages/roosterjs-content-model-plugins/lib/edit/keyboardTab.ts | Updates keyboardTab function to accept and check HandleTabOptions for each tab behavior |
| packages/roosterjs-content-model-plugins/lib/edit/tabUtils/handleTabOnList.ts | Adds blank line for code formatting |
| packages/roosterjs-content-model-plugins/lib/index.ts | Exports HandleTabOptions interface |
| packages/roosterjs-content-model-api/lib/modelApi/block/setModelIndentation.ts | Moves normalizeContentModel call here to ensure list cleanup after indentation changes |
| packages/roosterjs-content-model-api/lib/publicApi/block/setIndentation.ts | Removes normalizeContentModel call (moved to setModelIndentation) |
| packages/roosterjs-content-model-plugins/test/edit/keyboardTabTest.ts | Updates tests to pass DefaultHandleTabOptions to keyboardTab calls |
| packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts | Updates test expectations to use DefaultHandleTabOptions object instead of boolean |
| packages/roosterjs-content-model-plugins/test/edit/deleteSteps/deleteCollapsedSelectionTest.ts | Adds normalizeContentModel spy and fixes spelling in test descriptions ("Dont" → "Do not") |
| packages/roosterjs-content-model-api/test/modelApi/block/setModelIndentationTest.ts | Adds normalizeContentModel spy assertions to verify cleanup after indentation |
| demo/scripts/controlsV2/sidePane/editorOptions/Plugins.tsx | Updates UI to show individual checkboxes for each HandleTabOptions property |
| demo/scripts/controlsV2/sidePane/editorOptions/OptionState.ts | Updates type to expect HandleTabOptions object instead of boolean |
| demo/scripts/controlsV2/sidePane/editorOptions/EditorOptionsPlugin.ts | Initializes handleTabKey with HandleTabOptions object in initial state |
Comments suppressed due to low confidence (1)
packages/roosterjs-content-model-plugins/test/edit/EditPluginTest.ts:233
- Missing test coverage for partial HandleTabOptions configuration. The PR introduces the ability to pass an object like
{ handleTabKey: { indentParagraph: false } }to selectively disable specific tab behaviors while keeping others enabled. Consider adding a test case like:
it('Tab with partial options - only indentParagraph disabled', () => {
plugin = new EditPlugin({
handleTabKey: { indentParagraph: false }
});
const rawEvent = { key: 'Tab' } as any;
plugin.initialize(editor);
plugin.onPluginEvent({
eventType: 'keyDown',
rawEvent,
});
expect(keyboardTabSpy).toHaveBeenCalledWith(editor, rawEvent, {
indentMultipleBlocks: true,
indentTable: true,
appendTableRow: true,
indentList: true,
indentParagraph: false,
});
});This would verify that the option merging logic in the constructor (lines 66-71 of EditPlugin.ts) works correctly.
it('Tab, Tab handling not enabled', () => {
plugin = new EditPlugin({ handleTabKey: false });
const rawEvent = { key: 'Tab' } as any;
plugin.initialize(editor);
plugin.onPluginEvent({
eventType: 'keyDown',
rawEvent,
});
expect(keyboardTabSpy).toHaveBeenCalledWith(editor, rawEvent, {
indentMultipleBlocks: false,
indentTable: false,
appendTableRow: false,
indentList: false,
indentParagraph: false,
});
expect(keyboardInputSpy).not.toHaveBeenCalled();
expect(keyboardDeleteSpy).not.toHaveBeenCalled();
expect(keyboardEnterSpy).not.toHaveBeenCalled();
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /** | ||
| * @param options An optional parameter that takes in an object of type EditOptions, which includes the following properties: | ||
| * handleTabKey: A boolean that enables or disables Tab key handling. Defaults to true. |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation comment is outdated and doesn't reflect the new HandleTabOptions interface. It states "handleTabKey: A boolean that enables or disables Tab key handling" but now it can also accept a HandleTabOptions object with granular control over specific Tab behaviors. Consider updating to:
@param options An optional parameter that takes in an object of type EditOptions, which includes the following properties:
handleTabKey: A boolean or HandleTabOptions object that controls Tab key handling. When a boolean, true enables all features and false disables all. When an object, individual features can be controlled. Defaults to all enabled.
| * handleTabKey: A boolean that enables or disables Tab key handling. Defaults to true. | |
| * handleTabKey: A boolean or HandleTabOptions object that controls Tab key handling. When a boolean, true enables all features and false disables all. When an object, individual features can be controlled. Defaults to all enabled. |
| break; | ||
|
|
||
| case 'Tab': | ||
| if (this.options.handleTabKey && !hasCtrlOrMetaKey) { |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition if (this.options.handleTabKey && !hasCtrlOrMetaKey) is checking if handleTabKey is truthy, but now handleTabKey is always a Required object (never boolean) as defined in line 59. This check will always evaluate to true since objects are truthy.
The logic should check if at least one of the tab options is enabled, or this check should be removed entirely since keyboardTab itself will handle the options appropriately. Consider removing this condition or checking if any option is enabled:
if (!hasCtrlOrMetaKey) {
keyboardTab(editor, rawEvent, this.options.handleTabKey);
}| if (this.options.handleTabKey && !hasCtrlOrMetaKey) { | |
| if (!hasCtrlOrMetaKey) { |
|
|
||
| export type EditOptions = { | ||
| /** | ||
| * Whether to handle Tab key in keyboard. @default true |
Copilot
AI
Nov 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation comment for handleTabKey should be updated to reflect that it now accepts a HandleTabOptions object. Consider updating to:
/**
* Whether to handle Tab key in keyboard, or an object to control specific Tab key behaviors.
* When true, all Tab features are enabled. When false, all are disabled.
* When an object, individual features can be controlled via HandleTabOptions.
* @default true
*/
| * Whether to handle Tab key in keyboard. @default true | |
| * Whether to handle Tab key in keyboard, or an object to control specific Tab key behaviors. | |
| * When true, all Tab features are enabled. When false, all are disabled. | |
| * When an object, individual features can be controlled via HandleTabOptions. | |
| * @default true |
Accessibility: Keyboard Focus Trapped When handleTabKey is Enabled in RoosterJS v9 #3171
Shift tab not working when content has bullets #3184
normalizeContentModelto convert empty list item to be normal block.handleTabKeyas a boolean value, but this causes we cannot disable a single TAB feature so focus can be trapped in editor. To fix it, now we support disable single tab feature when create EditPlugin:This will still keep other features of TAB key enabled, but only disable indent paragraphs. So if focus is not in list or table, TAB key will move focus out of editor. The old way to pass in a boolean as handleTabKey is still available, and in that case true means all enabled, false means all disabled to keep it consistent with original behavior.