Clean up command factory #3431
Clean up command factory #3431
Conversation
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3431/ |
john-traas
force-pushed
the
clean-up-command-factory
branch
2 times, most recently
from
48cb384 to
c6a8a28
Compare
john-traas
force-pushed
the
clean-up-command-factory
branch
from
c6a8a28 to
24b1710
Compare
adrianschmidt
force-pushed
the
main
branch
3 times, most recently
from
197298f to
a0e1088
Compare
There was a problem hiding this comment.
Why move tests into a folder called __tests__? Is that a prosemirror convention? Because we don't use that pattern anywhere else in lime-elements… 🧐
There was a problem hiding this comment.
Ah... The initial folder/file was generated by cursor when I had it help me setup the boilerplate for the tests. We should follow the normal pattern we use and have the tests alongside the files they are testing.
|
If you try to make sub-lists, the behaviour quickly gets quite weird 🤔 Screen.Recording.2025-02-15.at.22.05.14.mov |
adrianschmidt
force-pushed
the
clean-up-command-factory
branch
from
ed54b1f to
fba00f3
Compare
I think I have some ideas of why this is happening. I'll take a look again on Monday. |
|
Yeah, I don't expect you to be working on the weekend just because I am 😂 No stress my friend! 🤗 |
john-traas
force-pushed
the
clean-up-command-factory
branch
2 times, most recently
from
8eb4802 to
07b8313
Compare
## [38.6.0](v38.5.0...v38.6.0) (2025-03-25) ### Features * **text editor:** conditionally allow commands ([819f599](819f599))
Bumps [@eslint/eslintrc](https://github.com/eslint/eslintrc) from 3.3.0 to 3.3.1. - [Release notes](https://github.com/eslint/eslintrc/releases) - [Changelog](https://github.com/eslint/eslintrc/blob/main/CHANGELOG.md) - [Commits](eslint/eslintrc@v3.3.0...v3.3.1) --- updated-dependencies: - dependency-name: "@eslint/eslintrc" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [@microsoft/api-extractor](https://github.com/microsoft/rushstack/tree/HEAD/apps/api-extractor) from 7.52.1 to 7.52.2. - [Changelog](https://github.com/microsoft/rushstack/blob/main/apps/api-extractor/CHANGELOG.md) - [Commits](https://github.com/microsoft/rushstack/commits/@microsoft/api-extractor_v7.52.2/apps/api-extractor) --- updated-dependencies: - dependency-name: "@microsoft/api-extractor" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
john-traas
force-pushed
the
clean-up-command-factory
branch
from
07b8313 to
2545fc2
Compare
📝 WalkthroughWalkthroughThis PR increments the project version to 38.6.0 and updates the changelog, package.json, and sonar configuration accordingly. It introduces new utility functions and refactors for the ProseMirror text editor—improving list command handling (including toggling between ordered and unordered lists), link validation, node and selection utilities, and menu state tracking. Additionally, a broad suite of new test files and documents has been added to support command testing, content generation, editor state/view building, event simulation, and schema creation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant CmdHandler as List Command Handler
participant Util as List Utility Functions
User->>Editor: Trigger toggle list command
Editor->>CmdHandler: Dispatch command request
CmdHandler->>Util: Check selection state (list vs. no selection)
Util-->>CmdHandler: Return list context details
CmdHandler->>Editor: Apply list type conversion (ordered ↔ unordered)
Editor-->>User: Updated document state
Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 33
🔭 Outside diff range comments (1)
src/components/text-editor/list-functionality-implementation-plan.md (1)
163-174: 🧹 Nitpick (assertive)Missing language specification for code block
The final code block is missing a language specification, which is required by markdown standards. Also, the code block appears to be incomplete as the implementation details are not shown.
- ``` + ```typescript + // Add implementation details for keyboard shortcuts + ```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
173-173: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
173-173: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (37)
CHANGELOG.md(1 hunks)package.json(2 hunks)sonar-project.properties(1 hunks)src/components/text-editor/list-functionality-development-log.md(1 hunks)src/components/text-editor/list-functionality-implementation-plan.md(1 hunks)src/components/text-editor/list-functionality.md(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/active-state-utils.ts(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/link-utils.ts(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/list-utils.ts(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/node-utils.ts(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/selection-utils.ts(1 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts(4 hunks)src/components/text-editor/prosemirror-adapter/menu/menu-list-commands.spec.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/link-plugin.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.spec.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.ts(3 hunks)src/components/text-editor/prosemirror-adapter/plugins/table-plugin.spec.ts(1 hunks)src/components/text-editor/prosemirror-adapter/plugins/table-plugin.ts(1 hunks)src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx(2 hunks)src/components/text-editor/test-setup/command-tester.spec.ts(1 hunks)src/components/text-editor/test-setup/command-tester.ts(1 hunks)src/components/text-editor/test-setup/content-generator.spec.ts(1 hunks)src/components/text-editor/test-setup/content-generator.ts(1 hunks)src/components/text-editor/test-setup/editor-state-builder.spec.ts(1 hunks)src/components/text-editor/test-setup/editor-state-builder.ts(1 hunks)src/components/text-editor/test-setup/editor-view-builder.spec.ts(1 hunks)src/components/text-editor/test-setup/editor-view-builder.ts(1 hunks)src/components/text-editor/test-setup/event-simulator.spec.ts(1 hunks)src/components/text-editor/test-setup/event-simulator.ts(1 hunks)src/components/text-editor/test-setup/schema-builder.spec.ts(1 hunks)src/components/text-editor/test-setup/schema-builder.ts(1 hunks)src/components/text-editor/test-setup/test-utils-development-log.md(1 hunks)src/components/text-editor/test-setup/test-utils.md(1 hunks)src/components/text-editor/test-setup/testing-utils-development-log.md(1 hunks)src/components/text-editor/test-setup/testing-utils-guide.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{ts,tsx}`: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.spec.tssrc/components/text-editor/prosemirror-adapter/menu/menu-list-commands.spec.tssrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/link-utils.tssrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/node-utils.tssrc/components/text-editor/test-setup/editor-view-builder.tssrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/selection-utils.tssrc/components/text-editor/test-setup/schema-builder.spec.tssrc/components/text-editor/test-setup/schema-builder.tssrc/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.spec.tssrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/active-state-utils.tssrc/components/text-editor/prosemirror-adapter/plugins/table-plugin.tssrc/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.tssrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/list-utils.tssrc/components/text-editor/test-setup/command-tester.spec.tssrc/components/text-editor/prosemirror-adapter/plugins/list-key-handler.tssrc/components/text-editor/test-setup/command-tester.tssrc/components/text-editor/prosemirror-adapter/plugins/link-plugin.tssrc/components/text-editor/test-setup/event-simulator.spec.tssrc/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.tssrc/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsxsrc/components/text-editor/test-setup/event-simulator.tssrc/components/text-editor/test-setup/editor-state-builder.tssrc/components/text-editor/test-setup/content-generator.tssrc/components/text-editor/test-setup/editor-state-builder.spec.tssrc/components/text-editor/test-setup/content-generator.spec.tssrc/components/text-editor/prosemirror-adapter/menu/menu-commands.tssrc/components/text-editor/test-setup/editor-view-builder.spec.ts
`**/*.tsx`: Our `.tsx` files are typically using StencilJS, not React.
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React.
src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
🧬 Code Definitions (16)
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.spec.ts (1)
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.ts (3)
getTableEditingPlugins(4-10)getTableNodes(22-31)createStyleAttribute(12-20)
src/components/text-editor/prosemirror-adapter/menu/menu-list-commands.spec.ts (1)
src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts (1)
createListCommand(305-347)
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/link-utils.ts (1)
src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts (1)
isExternalLink(108-110)
src/components/text-editor/test-setup/editor-view-builder.ts (2)
src/components/text-editor/test-setup/editor-state-builder.ts (1)
createEditorState(14-29)src/components/text-editor/test-setup/schema-builder.ts (1)
createTestSchema(12-33)
src/components/text-editor/test-setup/schema-builder.spec.ts (1)
src/components/text-editor/test-setup/schema-builder.ts (2)
createTestSchema(12-33)createCustomTestSchema(42-87)
src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.spec.ts (1)
src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.ts (2)
getMenuItemStates(20-35)createMenuStateTrackingPlugin(37-76)
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/active-state-utils.ts (1)
src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts (1)
CommandWithActive(34-37)
src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.ts (1)
src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.ts (4)
isInListItem(15-30)isEmptyListItem(35-55)isAtStartOfListItem(60-81)createListKeyHandlerPlugin(93-178)
src/components/text-editor/test-setup/command-tester.spec.ts (3)
src/components/text-editor/test-setup/editor-state-builder.ts (1)
createEditorState(14-29)src/components/text-editor/test-setup/command-tester.ts (4)
getCommandResult(29-60)testCommand(74-120)testCommandWithView(130-185)createCommandTester(193-202)src/components/text-editor/test-setup/editor-view-builder.ts (1)
cleanupEditorView(84-93)
src/components/text-editor/test-setup/command-tester.ts (1)
src/components/text-editor/test-setup/editor-view-builder.ts (1)
createEditorView(14-46)
src/components/text-editor/test-setup/event-simulator.spec.ts (3)
src/components/text-editor/test-setup/editor-state-builder.ts (1)
createEditorState(14-29)src/components/text-editor/test-setup/editor-view-builder.ts (2)
createEditorView(14-46)cleanupEditorView(84-93)src/components/text-editor/test-setup/event-simulator.ts (6)
simulateKeyPress(63-84)KeyModifiers(48-53)PasteData(89-93)simulatePaste(102-139)simulateClick(152-191)simulateDragAndDrop(204-288)
src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (1)
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.ts (1)
getTableEditingPlugins(4-10)
src/components/text-editor/test-setup/editor-state-builder.ts (1)
src/components/text-editor/test-setup/schema-builder.ts (1)
createTestSchema(12-33)
src/components/text-editor/test-setup/editor-state-builder.spec.ts (2)
src/components/text-editor/test-setup/editor-state-builder.ts (4)
createEditorState(14-29)createEditorStateWithSelection(41-59)setTextSelection(69-77)createDocumentWithText(100-107)src/components/text-editor/test-setup/schema-builder.ts (1)
createCustomTestSchema(42-87)
src/components/text-editor/test-setup/content-generator.spec.ts (2)
src/components/text-editor/test-setup/content-generator.ts (8)
createDocWithText(13-18)createDocWithHTML(27-31)MarkSpec(36-39)createDocWithFormattedText(49-61)createDocWithBulletList(97-118)createDocWithHeading(128-144)createDocWithBlockquote(153-166)createDocWithCodeBlock(175-187)src/components/text-editor/test-setup/schema-builder.ts (1)
createCustomTestSchema(42-87)
src/components/text-editor/test-setup/editor-view-builder.spec.ts (2)
src/components/text-editor/test-setup/editor-view-builder.ts (4)
cleanupEditorView(84-93)createEditorView(14-46)createDispatchSpy(54-75)mockProseMirrorDOMEnvironment(101-143)src/components/text-editor/test-setup/editor-state-builder.ts (1)
createEditorState(14-29)
🪛 GitHub Actions: Pull Request Checks
src/components/text-editor/prosemirror-adapter/menu/menu-list-commands.spec.ts
[error] 152-152: Expected: 'ordered_list', Received: 'bullet_list'
[error] 262-262: Expected: 'paragraph', Received: 'bullet_list'
[error] 386-386: Expected: 'bullet_list', Received: 'paragraph'
src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.ts
[error] 106-106: Expected: true, Received: false
[error] 435-435: Expected number of calls: >= 1, Received number of calls: 0
[error] 476-476: Expected number of calls: >= 1, Received number of calls: 0
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
3-3: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
6-6: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
src/components/text-editor/list-functionality-implementation-plan.md
62-62: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
103-103: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
138-138: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
173-173: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
173-173: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
src/components/text-editor/test-setup/testing-utils-guide.md
12-12: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
12-12: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
13-13: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
16-16: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
17-17: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
20-20: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
21-21: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
24-24: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
25-25: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
27-27: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
30-30: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
30-30: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
31-31: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
34-34: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
35-35: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
38-38: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
38-38: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
39-39: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
41-41: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3
(MD029, ol-prefix)
45-45: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
49-49: Ordered list item prefix
Expected: 3; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
54-54: Ordered list item prefix
Expected: 4; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
60-60: Ordered list item prefix
Expected: 5; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
62-62: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
65-65: Ordered list item prefix
Expected: 6; Actual: 7; Style: 1/2/3
(MD029, ol-prefix)
69-69: Ordered list item prefix
Expected: 7; Actual: 8; Style: 1/2/3
(MD029, ol-prefix)
🪛 Biome (1.9.4)
src/components/text-editor/test-setup/editor-view-builder.ts
[error] 90-90: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/list-utils.ts
[error] 124-124: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/components/text-editor/test-setup/editor-state-builder.ts
[error] 101-101: This type annotation is trivially inferred from its initialization.
Safe fix: Remove the type annotation.
(lint/style/noInferrableTypes)
src/components/text-editor/test-setup/content-generator.ts
[error] 130-130: This type annotation is trivially inferred from its initialization.
Safe fix: Remove the type annotation.
(lint/style/noInferrableTypes)
src/components/text-editor/test-setup/content-generator.spec.ts
[error] 119-119: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/components/text-editor/test-setup/editor-view-builder.spec.ts
[error] 210-210: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 211-211: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 LanguageTool
src/components/text-editor/test-setup/test-utils-development-log.md
[style] ~112-~112: Consider an alternative adverb to strengthen your wording.
Context: ...anupEditorView(view, container?)** - Properly cleans up the editor view to prevent me...
(PROPERLY_THOROUGHLY)
[style] ~164-~164: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...to a state - Returns a CommandResult with success status, transaction, and new state -...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[uncategorized] ~189-~189: The preposition ‘as’ seems more likely in this position.
Context: ...ers?)** - Simulates key press events on the editor - Supports modifier keys ...
(AI_HYDRA_LEO_REPLACE_ON_AS)
src/components/text-editor/test-setup/testing-utils-development-log.md
[uncategorized] ~77-~77: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ting - Accepts optional HTML content string - Uses the test schema by default, o...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~112-~112: Consider an alternative adverb to strengthen your wording.
Context: ...anupEditorView(view, container?)** - Properly cleans up the editor view to prevent me...
(PROPERLY_THOROUGHLY)
[style] ~164-~164: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...to a state - Returns a CommandResult with success status, transaction, and new state -...
(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[uncategorized] ~189-~189: The preposition ‘as’ seems more likely in this position.
Context: ...ers?)** - Simulates key press events on the editor - Supports modifier keys ...
(AI_HYDRA_LEO_REPLACE_ON_AS)
src/components/text-editor/list-functionality-development-log.md
[uncategorized] ~82-~82: You might be missing the article “the” here.
Context: ...lows the core principles established in implementation plan ### [March 26, 2024] **Fixed Lis...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~95-~95: You might be missing the article “the” here.
Context: ...e Behavior** - Fixed a critical bug in handleListWithSelection function in `me...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[style] ~97-~97: Consider using a different verb for a more formal wording.
Context: ...fferent list types (bullet/ordered) - Fixed the issue with toggling list off back t...
(FIX_RESOLVE)
🔇 Additional comments (110)
sonar-project.properties (1)
4-4: Version Bump Consistency Check
The update tosonar.projectVersion=38.6.0correctly reflects the new release version. This change ensures consistency across the metadata in your project, aligning with updates inpackage.jsonand the changelog.src/components/text-editor/prosemirror-adapter/plugins/link-plugin.ts (1)
5-7: Import refactoring looks good.The
isValidUrlfunction has been moved frommenu-commandsto a more specific utility filemenu-command-utils/link-utils. This improves code organization by placing utility functions in dedicated files based on their functionality.package.json (2)
3-3: Version increment is appropriate.Incrementing to version 38.6.0 aligns with the feature changes described in the PR objectives for improving command factory functionality.
47-49: Dependency updates are appropriate.Minor version updates to development dependencies help keep the project current with bug fixes and improvements.
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.ts (1)
12-20: Function export is appropriate.Making
createStyleAttributeexportable allows it to be reused across modules, which is good for code reusability. The implementation remains unchanged and maintains its original functionality.🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/node-utils.ts (1)
1-22: Well-implemented utility function.The
findAncestorDepthOfTypeutility is well-structured with:
- Clear TypeScript typing
- Comprehensive JSDoc documentation
- Efficient implementation that iterates through ancestor nodes
- Proper null return when no matching ancestor is found
This will be useful for determining the structure of the document when handling commands, especially for list operations mentioned in the PR objectives.
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/selection-utils.ts (1)
11-17: LGTM! Clean and straightforward implementation.This utility function correctly creates a new TextSelection using resolved document positions.
src/components/text-editor/test-setup/schema-builder.ts (2)
6-33: LGTM! Well-structured schema creation function with good documentation.The
createTestSchemafunction is well-documented and correctly creates a standardized ProseMirror schema with all the necessary nodes and marks for testing the text editor.
35-87: LGTM! Flexible schema customization with comprehensive options.The
createCustomTestSchemafunction provides excellent flexibility for creating custom schemas with various configurations. The implementation is clean, well-organized, and properly handles all the provided options.src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (3)
55-55: LGTM! New import for list key handler plugin.This import adds the necessary plugin for enhanced keyboard handling with lists.
402-402: LGTM! Added list key handler plugin to the editor.The plugin is correctly integrated into the editor's plugin array, which will enable special key handling for list operations.
408-427: LGTM! Enhanced action bar filtering based on allowed types.The implementation now correctly filters action bar items based on whether they are allowed in the current context, which aligns with the PR's objective of cleaning up the command factory.
The changes to this method will help address the issue with sub-lists mentioned in the PR comments by giving better control over what list operations are available in different contexts.
src/components/text-editor/test-setup/schema-builder.spec.ts (2)
4-30: LGTM! Thorough tests for the createTestSchema function.The tests properly verify that the schema created by
createTestSchemaincludes all the expected nodes and marks, providing good test coverage for this utility.
32-79: LGTM! Comprehensive tests for createCustomTestSchema with various configurations.These tests effectively verify the customizability of the schema builder, including options to exclude certain features and add custom marks and nodes. The test coverage is thorough and well-structured.
src/components/text-editor/test-setup/testing-utils-development-log.md (1)
1-220: Documentation is thorough and well-structuredThis development log provides excellent documentation of the testing utilities implementation. The clear organization with planned implementations, completed work (with dates), and implementation notes makes this a valuable reference for developers working on the text editor component.
The checklist approach with detailed explanations of each utility's purpose and parameters will help maintain consistency and encourage reuse of these testing utilities.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...ting - Accepts optional HTML content string - Uses the test schema by default, o...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~112-~112: Consider an alternative adverb to strengthen your wording.
Context: ...anupEditorView(view, container?)** - Properly cleans up the editor view to prevent me...(PROPERLY_THOROUGHLY)
[style] ~164-~164: ‘with success’ might be wordy. Consider a shorter alternative.
Context: ...to a state - Returns a CommandResult with success status, transaction, and new state -...(EN_WORDINESS_PREMIUM_WITH_SUCCESS)
[uncategorized] ~189-~189: The preposition ‘as’ seems more likely in this position.
Context: ...ers?)** - Simulates key press events on the editor - Supports modifier keys ...(AI_HYDRA_LEO_REPLACE_ON_AS)
src/components/text-editor/test-setup/command-tester.spec.ts (5)
1-10: LGTM: Well-structured importsThe imports are organized properly and use relative paths as required by the coding guidelines.
11-50: Comprehensive tests for getCommandResultThese tests thoroughly validate both the success and failure paths of the getCommandResult function, including checking transaction creation and state updates.
52-114: Well-structured tests for testCommandThe tests for the testCommand function cover all essential aspects: applicability verification, content verification, and document size changes. The explanatory comments enhance readability.
116-172: Good practice with resource cleanupThe test properly manages the EditorView lifecycle by tracking the view and container for cleanup in the afterEach block, which prevents memory leaks.
174-209: Thorough testing of createCommandTesterThis test effectively validates that the createCommandTester function produces reusable testers that maintain correct behavior across different states.
src/components/text-editor/list-functionality-implementation-plan.md (3)
1-50: Well-organized implementation planThis document clearly outlines the phased approach to enhancing list functionality while maintaining backward compatibility. The core principles emphasize preserving existing behavior, which is crucial for maintaining stability.
51-96: Detailed implementation approach for Tab/Shift+Tab handlingThe implementation approach for indenting/outdenting list items is well-documented with clear code examples.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
62-62: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
128-162: Clear backspace behavior implementationThe implementation details for improved backspace behavior are thorough and cover important edge cases like joining with non-list content.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
138-138: Fenced code blocks should be surrounded by blank lines
null(MD031, blanks-around-fences)
src/components/text-editor/prosemirror-adapter/menu/menu-list-commands.spec.ts (2)
1-6: LGTM: Imports use relative paths as requiredThe imports correctly use relative paths following the coding guidelines.
7-55: Well-structured test setup and helper functionsThe test suite has excellent organization with clear helper functions for creating paragraphs, selecting content, and verifying list structures.
src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.spec.ts (8)
1-9: LGTM - Imports and setup look goodThe import statements are correctly using relative paths, adhering to the project's coding guidelines. The file imports the necessary functions from ProseMirror and local modules.
10-15: Appropriate mock data setupGood use of mocking the
EditorMenuTypesenum for isolated testing purposes.
17-42: Well-structured test setupThe test setup is comprehensive with proper mocking of the
MenuCommandFactory,updateCallback,view, andstate. The use of Jest mocks ensures isolated testing of the target functionality.
44-87: Thorough testing of getMenuItemStatesThe first test case properly verifies that active and allowed states are correctly determined from command properties. Good validation of method calls with
toHaveBeenCalledWith.
89-122: Edge case testing for missing methodsGood coverage of the edge cases where active or allowed methods are missing. The test verifies that sensible defaults are used (false for active, true for allowed) when methods are not present.
125-135: Plugin creation verificationThe test correctly verifies that the plugin is created with the expected instance type and key.
137-160: State update testingThe test effectively validates the plugin's state management when metadata is present in a transaction.
162-181: No-change scenario testingThis test correctly verifies that the plugin preserves the existing state when no metadata is provided in a transaction.
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/link-utils.ts (2)
1-2: Imports look goodThe imports are using relative paths as required by the coding guidelines.
4-12: Effective URL validationThe
isValidUrlfunction is implemented correctly using try-catch with the URL constructor for validation.src/components/text-editor/list-functionality.md (4)
1-25: Well-structured documentation of current list featuresThe documentation clearly outlines the current capabilities of list functionality in the text editor, which serves as a good reference point for understanding the existing implementation.
26-52: Comprehensive roadmap for planned featuresThe checklist format for planned features is excellent for tracking progress. The scope covers all important aspects of list functionality, from basic operations to advanced features like custom list markers and accessibility improvements.
54-78: Thorough implementation considerationsThe documentation outlines important technical considerations for the implementation, including schema structure, commands, key bindings, testing requirements, and user experience considerations.
80-96: Clear technical approachThe technical approach section provides a solid foundation for implementing the planned features, with specific considerations for ProseMirror plugins, command enhancements, schema modifications, and input rules.
src/components/text-editor/prosemirror-adapter/plugins/table-plugin.spec.ts (5)
1-15: LGTM - Imports and setupThe imports are correctly using relative paths as required by the coding guidelines, and all necessary utilities are imported for testing.
16-26: Good test cleanupProper cleanup after each test to prevent test pollution and memory leaks.
28-70: Comprehensive testing of table plugin enablingThe tests thoroughly verify the plugin behavior when tables are enabled or disabled, and confirm that the plugin can be added to an editor state.
72-116: Complete coverage of table node specificationsThe tests verify that all necessary table node types are defined and can be added to a schema, including custom cell attributes for styling.
118-168: Thorough style attribute helper testingGood coverage of the style attribute helper, including getting values from DOM elements, setting values on attributes objects, and handling edge cases like falsy values.
src/components/text-editor/list-functionality-development-log.md (2)
1-26: Well-structured development log with clear implementation phasesThis document effectively outlines the development approach for list functionality with clear phases and verification requirements. The critical warning about maintaining existing functionality is appropriately highlighted.
96-98: Good documentation of the critical bug fixThe explanation of the list toggle behavior fix is clear and covers all the key aspects of the issue resolution.
🧰 Tools
🪛 LanguageTool
[style] ~97-~97: Consider using a different verb for a more formal wording.
Context: ...fferent list types (bullet/ordered) - Fixed the issue with toggling list off back t...(FIX_RESOLVE)
src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.ts (1)
522-530: Clever approach to testing preventDefaultThis is a good technique for testing whether preventDefault was called on the event, by overriding the method and tracking its invocation.
src/components/text-editor/test-setup/test-utils-development-log.md (2)
1-52: Comprehensive test utilities documentation with clear implementation statusThis document provides an excellent overview of the planned and implemented test utilities with clear checkboxes indicating completion status. The organization into categories makes it easy to navigate.
211-220: Well-documented implementation notesThese notes provide good context about the consistency and integration of the test utilities. They will be helpful for developers using the testing suite.
src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/active-state-utils.ts (3)
5-18: Well-implemented mark active state checkerThis utility correctly handles both empty and non-empty selections when checking if a mark is active. For empty selections, it looks at the stored marks or marks at the cursor position, while for non-empty selections it checks if the mark exists in the selected range.
19-38: Good handling of node types and special casesThe function properly checks if the current node matches the specified type and handles the special case for headings with level attributes. The depth-based node lookup is the correct approach.
1-4:❓ Verification inconclusive
Check imports for compliance with coding guidelines
According to the coding guidelines, imports from other files in the same module (lime-elements) must use relative paths.
The current imports look like relative paths, which is good. Let's verify this is consistent across the codebase:
🏁 Script executed:
#!/bin/bash # Check if there are any absolute imports in the TS files for lime-elements rg -l "from 'lime-elements" --type tsLength of output: 42
Import paths verified for compliance.
The local module imports insrc/components/text-editor/prosemirror-adapter/menu/menu-command-utils/active-state-utils.tsuse relative paths as required by our coding guidelines, and the external import from'prosemirror-model'is acceptable. However, the repository-wide search for absolute imports referencinglime-elementsproduced no output—which is inconclusive. Please manually verify across the codebase that no module-internal imports use absolute paths.src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.ts (3)
1-30: Good initial checks for determining if the cursor is within a list item.No syntax or logical errors found. The
isInListItemfunction accurately traverses up the node hierarchy to detect alist_item. This helps ensure that subsequent list-related commands are only executed in the correct context.
57-81: Straightforward logic for determining cursor position at the start of a list item.The
isAtStartOfListItemfunction is well-structured. It returns early for non-collapsed selections and checks whether the cursor is exactly at the start of thelist_item. This code block appears correct for the intended functionality.
83-178:✅ Verification successful
Comprehensive key handling for list items with standard list commands.
The plugin design covers Tab/Shift+Tab for (out)denting, Enter for splitting or exiting empty list items, and Backspace for joining with the previous item or lifting out of the list. This is consistent with typical editor behavior.
However, given the reported sub-list “weird” behavior, consider adding tests for nested lists to ensure correct handling.
Here is a script that searches for sub-list usage in the tests or code, which can help verify that nesting is handled. If sub-lists appear or your plugin references them, you can confirm coverage:
🏁 Script executed:
#!/bin/bash # Searches for potential references to sub-lists in the codebase rg -A 5 "nested list" rg -A 5 "sub-list" rg -A 5 "list.*list"Length of output: 67012
Verified: Nested List Behavior is Fully Tested and Functions as Expected
The existing tests in the codebase—especially within
src/components/text-editor/prosemirror-adapter/plugins/list-key-handler.spec.ts—demonstrate that nested and sub-list scenarios (including indenting and outdenting) are being validated. No additional tests or changes are necessary at this time.src/components/text-editor/test-setup/editor-view-builder.ts (4)
14-46: Solid approach for creating and returning an editor view with container management.This function is elegantly structured, providing both a default container and a default
EditorStateif none are supplied. Attaching the container to the DOM conditionally prevents extraneous elements during test runs. The code is clear and maintainable.
54-75: Flexible dispatch spy for transaction updates.Automatically applying transactions to the view’s state is a neat solution for simplifying test flows. The
setViewmethod is especially useful for capturing the created view reference. No issues found here.
84-94: Proper cleanup routine to avoid memory leaks.Destroying the view is essential to free resources, and removing the container from the DOM is a good practice. No concerns here.
🧰 Tools
🪛 Biome (1.9.4)
[error] 90-90: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
101-143: Minimal DOM environment mocking for Node-based tests.This utility is effective for running ProseMirror-based tests without a full browser environment. The logic to restore the original
windowanddocumentobjects is correct. Good job ensuring that the mock environment remains confined to the test scope.src/components/text-editor/test-setup/editor-state-builder.ts (2)
14-29: Verify error handling for invalid or unparseable HTML content
IncreateEditorState, ifparseContentToDocfails to parse invalid HTML, the method may throw an unhandled error. Consider a fallback or explicit error handling logic.
86-91: Security note for HTML parsing
Though this is test code, usinginnerHTMLcould be an attack vector if untrusted HTML were ever introduced. For production-level code, consider secure HTML parsing or sanitization.src/components/text-editor/test-setup/event-simulator.ts (5)
8-43: Well-structured mock implementation
MockDataTransferproperly simulates file addition and data handling for testing. The mutable map approach is clear and sufficient for test environments. No immediate issues found.
63-84: Confirm cross-browser consistency
simulateKeyPressdispatches a'keydown'event usingKeyboardEvent. Some test environments may not fully supportKeyboardEventInitfields. Verify that your test runners handle them consistently.
102-139: Ensure HTML paste content is handled as expected
simulatePastesets both'text/plain'and'text/html'if provided. Confirm that ProseMirror's paste handling merges or prioritizes these as intended, especially if the test environment differs from a real browser.
152-191: Mock coordinate-based interactions
simulateClickgracefully assigns default bounding client rect and mockselementFromPoint. This is appropriate for typical test environments. No changes necessary.
204-288: Robust drag-and-drop simulation
simulateDragAndDropgenerates multiple events (mousedown, dragstart, dragover, drop, mouseup). This resembles real usage closely enough for thorough test coverage. Looks good.src/components/text-editor/test-setup/command-tester.ts (4)
16-20: Interface helps clarify command outcomes
CommandResultis clean and self-explanatory. This aids readability when checking commands’ side effects during tests.
74-120: Comprehensive verification of command behavior
testCommandchecks multiple aspects (applicability, text content, doc size, etc). This thorough approach is beneficial. Ensure that tests using it also cover commands that partially modify the doc (e.g., toggling marks).
130-185: DOM-based command validation
testCommandWithViewis a good extension for commands needing anEditorView. The approach is straightforward and well-structured.
193-202: Reusable command tester
createCommandTesterkeeps the test code DRY and encapsulates logic nicely. No issues found.src/components/text-editor/prosemirror-adapter/menu/menu-commands.ts (4)
5-5: Confirm usage of schema-list imports
The imports from'prosemirror-schema-list'(wrapInList,sinkListItem) are used for advanced list operations. They look correct for the toggling logic.
258-296: Streamlined approach to list toggling
handleListWithSelectioncaptures multiple list scenarios (removal, conversion, new wrap). The logic is well-structured. Just confirm thorough test coverage for selection ranges spanning multiple list items.
305-347: createListCommand handles single-cursor vs multi-range
Splitting logic intohandleListNoSelectionvs.handleListWithSelectionis a neat approach. Ensure conversions (bullet ↔ ordered) are tested with partially selected lists.
409-410: Added keyboard shortcuts
'Mod-Shift-8'and'Mod-Shift-9'for bullet/ordered lists are consistent with typical text editors. Implementation looks correct.src/components/text-editor/prosemirror-adapter/plugins/menu-state-tracking-plugin.ts (5)
10-13: Separation of active and allowed states looks clean
Defining bothactiveandallowedproperties in this interface provides a clear and extensible structure for tracking menu state.
15-18: Callback refactor
Separating the callback parameters intoactiveTypesandallowedTypesenhances clarity and type safety for external consumers.
25-26: Comprehensive state determination
Using separate records foractiveTypesandallowedTypesis an effective pattern, ensuring clarity when populating both properties.Also applies to: 34-34
46-46: Initializing empty allowed and active states
Returning empty records here provides a straightforward initial value before the plugin updates them.
69-71: Clear separation for callback arguments
Invoking theupdateCallbackwith separatemenuItemStates.activeandmenuItemStates.allowedkeeps the logic consistent.src/components/text-editor/test-setup/event-simulator.spec.ts (7)
18-30: Consistent setup in beforeEach
Creating a fresh editor view and attaching its container to the DOM for each test ensures isolation and prevents cross-test pollution.
32-41: Clean teardown in afterEach
Destroying the editor view and removing the container fosters best practices for test cleanup.
43-59: Key press simulation test
Verifying the generatedKeyboardEventand key property ensures correctness of the simulated key press utility.
61-83: Modifier keys test
Including tests for modifier keys (ctrl,shift, etc.) provides confidence in complex keyboard interactions.
86-133: Paste simulation tests
Handling both text and HTML content in these tests thoroughly validates thesimulatePasteutility.
135-171: Click simulation tests
Verifying mouse button codes (button= 0, 2) and click detail states (single/double) ensures robust coverage of mouse interactions.
173-233: Drag and drop simulation test
Checking the dispatch of multiple mouse and custom drag events in sequence solidly confirms that complex interactions are handled.src/components/text-editor/test-setup/editor-state-builder.spec.ts (4)
10-55: Thorough tests for createEditorState
Verifying default schema, content creation, and custom schema support ensures that editor states are controlled and flexible.
57-70: Selection creation tests
Ensuring that the editor state sets the expectedfromandtoselection boundaries adds confidence in selection-specific utilities.
72-84: Testing setTextSelection
Confirming that a new state is returned and that selection boundaries match expectations guards against accidental state mutation.
86-114: Document creation tests
Validating both text and empty paragraphs confirms thatcreateDocumentWithTexthandles content consistently for various scenarios.src/components/text-editor/prosemirror-adapter/menu/menu-command-utils/list-utils.ts (7)
1-11: Enumeration of list types
DefiningLIST_TYPESas a tuple closely couples it withEditorMenuTypesand clarifies valid list types at a glance.
12-25: Valid in-list detection
Iterating up through$from.depthis a typical approach for checking node ancestry. This should reliably detect the specified list type.
49-49: Defined dispatch type
Exporting a dedicatedDispatchtype clarifies the expected transaction signature for these utilities.
51-100: Controlled removal of list nodes
Replacing targeted list nodes with paragraphs while preserving content helps maintain consistent document flow.
133-172: Batch conversion of list nodes
Traversing the range vianodesBetweenand selectively creating new nodes ensures consistent transformation for an entire selection.
174-205: Toggling list logic
ApplyingfindWrappingandliftTargetcovers both wrapping new lists and lifting existing ones, maximizing reusability.
208-243: Single-list node conversion
Scanning upward through the selection depth until the matching list type is found helps ensure the correct node is replaced.src/components/text-editor/test-setup/content-generator.ts (4)
1-40: Well-structured utility functions with clear documentation.The content generator utility functions are well-documented with JSDoc comments that clearly explain their purpose, parameters, and return values. The code organization is clean and follows a consistent pattern.
41-88: Well-implemented mark application logic with proper error handling.The
createDocWithFormattedTextand its helper functioncreateTextNodeWithMarkscorrectly implement mark application with proper error handling for invalid mark types.
90-118: Bullet list creation logic is correctly implemented.The
createDocWithBulletListfunction correctly maps text items to list items and wraps them in a bullet list node. The implementation follows ProseMirror's document structure requirements.
120-144: Well-implemented specialized document creators.The heading, blockquote, and code block document creators all follow a consistent pattern and correctly implement the expected document structure for each type.
Also applies to: 146-166, 168-187
🧰 Tools
🪛 Biome (1.9.4)
[error] 130-130: This type annotation is trivially inferred from its initialization.
Safe fix: Remove the type annotation.
(lint/style/noInferrableTypes)
src/components/text-editor/test-setup/content-generator.spec.ts (3)
1-41: Well-structured test suite with thorough test cases.The test suite is well-organized with comprehensive test cases for the content generator utilities. Each test verifies both the returned state type and the document structure.
74-140: Thorough test coverage for formatted text functionality.The tests for
createDocWithFormattedTextcover all important aspects including applying different types of marks, handling mark attributes, and error cases for invalid mark types.🧰 Tools
🪛 Biome (1.9.4)
[error] 119-119: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
142-179: Comprehensive tests for document creation functions.The tests for bullet lists, headings, blockquotes, and code blocks thoroughly verify both the structure and content of the created documents, including edge cases like empty lists.
Also applies to: 181-254
src/components/text-editor/test-setup/editor-view-builder.spec.ts (3)
11-23: Good practice with proper cleanup in afterEach.The use of
afterEachto clean up the editor view and container prevents memory leaks, which is essential when testing components that manipulate the DOM.
79-137: Well-structured tests for dispatch spy functionality.The tests for
createDispatchSpythoroughly verify both the tracking of transactions and the conditional auto-update behavior.
139-190: Thorough testing of cleanup and DOM environment mocking.The tests for
cleanupEditorViewandmockProseMirrorDOMEnvironmentproperly verify the functionality using spies and appropriate assertions.Also applies to: 192-248
src/components/text-editor/test-setup/test-utils.md (3)
1-40: Comprehensive documentation of testing utilities.The documentation provides a clear overview of the available testing utilities with well-structured categories and descriptions.
41-74: Well-documented API reference.The utility functions are clearly listed with their signatures and purposes, making it easy for developers to understand what's available and how to use each utility.
75-191: Helpful usage examples for common testing scenarios.The examples demonstrate how to use the testing utilities in real-world scenarios, including setup/teardown patterns, command testing, and event simulation. These will be valuable for developers writing tests.
| @@ -1,3 +1,11 @@ | |||
| ## [38.6.0](https://github.com/Lundalogik/lime-elements/compare/v38.5.0...v38.6.0) (2025-03-25) | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Heading Level Concern
The first line is a level-2 heading (## [38.6.0]…). Markdownlint (MD041) expects the file’s first heading to be a top-level heading. If your project’s style guidelines require a top-level (#) heading at the start of the file, consider updating it accordingly, or alternatively, suppress this warning if the current style is intentional.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
1-1: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove Extra Blank Lines After Version Heading
Markdownlint (MD012) flags multiple consecutive blank lines after the version header. Please remove the extra blank line(s) to improve readability and comply with the expected single blank line spacing.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reduce Consecutive Blank Lines Before “Features” Section
Similarly, there are multiple consecutive blank lines before the “### Features” heading. Reducing these extra lines to a single blank line will enhance the document’s clarity.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
| export const adjustSelectionToFullBlocks = (state: EditorState) => { | ||
| const { $from, $to } = state.selection; | ||
| const from = $from.pos === $from.start() ? $from.pos : $from.end(); | ||
| const to = $to.pos === $to.end() ? $to.pos : $to.start(); | ||
|
|
||
| return { from: from, to: to }; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider adding clarification on the selection adjustment logic.
The adjustSelectionToFullBlocks function seems to do the opposite of what its name suggests in some cases. If the selection doesn't start at the beginning of a block, it uses the end position ($from.end()), and if it doesn't end at the end of a block, it uses the start position ($to.start()). This logic is counterintuitive and could benefit from a comment explaining this design choice.
export const adjustSelectionToFullBlocks = (state: EditorState) => {
const { $from, $to } = state.selection;
+ // If selection starts at the beginning of a block, use that position,
+ // otherwise use the end of the block containing the start position
const from = $from.pos === $from.start() ? $from.pos : $from.end();
+ // If selection ends at the end of a block, use that position,
+ // otherwise use the start of the block containing the end position
const to = $to.pos === $to.end() ? $to.pos : $to.start();
return { from: from, to: to };
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const adjustSelectionToFullBlocks = (state: EditorState) => { | |
| const { $from, $to } = state.selection; | |
| const from = $from.pos === $from.start() ? $from.pos : $from.end(); | |
| const to = $to.pos === $to.end() ? $to.pos : $to.start(); | |
| return { from: from, to: to }; | |
| }; | |
| export const adjustSelectionToFullBlocks = (state: EditorState) => { | |
| const { $from, $to } = state.selection; | |
| // If selection starts at the beginning of a block, use that position, | |
| // otherwise use the end of the block containing the start position | |
| const from = $from.pos === $from.start() ? $from.pos : $from.end(); | |
| // If selection ends at the end of a block, use that position, | |
| // otherwise use the start of the block containing the end position | |
| const to = $to.pos === $to.end() ? $to.pos : $to.start(); | |
| return { from: from, to: to }; | |
| }; |
| 1. **Split List Items:** | ||
| - Override the Enter key behavior within lists | ||
| - Implement `splitListItem` command to properly split the current list item | ||
| - Handle empty list items (create a new paragraph outside the list) | ||
|
|
||
| 2. **Implementation Details:** | ||
| ```typescript | ||
| handleEnter(view, event) { | ||
| const { state } = view; | ||
| const { selection } = state; | ||
| const { $from, empty } = selection; | ||
|
|
||
| // Check if cursor is in a list item | ||
| if (isInListItem(state)) { | ||
| // If the list item is empty, exit the list | ||
| if (isEmptyListItem(state)) { | ||
| return liftListItem(schema.nodes.list_item)(state, view.dispatch); | ||
| } | ||
|
|
||
| // Otherwise split the list item | ||
| return splitListItem(schema.nodes.list_item)(state, view.dispatch); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| ``` | ||
|
|
||
| 3. **Helper Functions:** | ||
| - Create an `isEmptyListItem` function to check if the current list item is empty | ||
| - Implement proper handling of cursor position after splitting | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add blank lines around code blocks
The fenced code blocks should be surrounded by blank lines for better readability according to Markdown standards.
- ```typescript
+
+ ```typescript
handleEnter(view, event) {
const { state } = view;
const { selection } = state;
const { $from, empty } = selection;
// Check if cursor is in a list item
if (isInListItem(state)) {
// If the list item is empty, exit the list
if (isEmptyListItem(state)) {
return liftListItem(schema.nodes.list_item)(state, view.dispatch);
}
// Otherwise split the list item
return splitListItem(schema.nodes.list_item)(state, view.dispatch);
}
return false;
}
- ```
+ ```
+🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
103-103: Fenced code blocks should be surrounded by blank lines
null
(MD031, blanks-around-fences)
| /** | ||
| * Get the other list type from the current list type. | ||
| * @param schema - The schema to use. | ||
| * @param currentType - The current list type. | ||
| * @returns The other list type. | ||
| */ | ||
| export const getOtherListType = ( | ||
| schema: Schema, | ||
| currentType: string, | ||
| ): NodeType => { | ||
| if (!LIST_TYPES.includes(currentType as any)) { | ||
| console.error(`Invalid list type: ${currentType}`); | ||
| } | ||
|
|
||
| const otherType = LIST_TYPES.find((type) => type !== currentType); | ||
| if (!otherType || !schema.nodes[otherType]) { | ||
| console.error(`List type "${otherType}" not found in schema`); | ||
| } | ||
|
|
||
| return schema.nodes[otherType]; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fallback logging using console.error
Using console.error here surfaces misconfiguration issues on the console.
Consider substituting a more robust logging mechanism or a user-facing warning if these errors require handling beyond development.
| const fromOrderedToBulletList = (fromType: NodeType, toType: NodeType) => { | ||
| return ( | ||
| fromType.name === EditorMenuTypes.OrderedList && | ||
| toType.name === EditorMenuTypes.BulletList | ||
| ); | ||
| }; | ||
|
|
||
| const fromBulletToOrderedList = (fromType: NodeType, toType: NodeType) => { | ||
| return ( | ||
| fromType.name === EditorMenuTypes.BulletList && | ||
| toType.name === EditorMenuTypes.OrderedList | ||
| ); | ||
| }; | ||
|
|
||
| const convertListAttributes = ( | ||
| fromType: NodeType, | ||
| toType: NodeType, | ||
| attrs: Record<string, any>, | ||
| ) => { | ||
| const newAttrs = { ...attrs }; | ||
| if (fromOrderedToBulletList(fromType, toType)) { | ||
| // Bullet lists generally do not need an "order" attribute. | ||
| delete newAttrs.order; | ||
| } else if (fromBulletToOrderedList(fromType, toType)) { | ||
| // For ordered lists, set a default start if not present. | ||
| newAttrs.order = newAttrs.order || 1; | ||
| } | ||
|
|
||
| return newAttrs; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Handling list attribute conversion
In convertListAttributes, using delete newAttrs.order; adheres to the expected logic but can be less performant.
To address the static analysis hint and potentially improve performance, update the attribute removal like this:
- delete newAttrs.order;
+ newAttrs.order = undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fromOrderedToBulletList = (fromType: NodeType, toType: NodeType) => { | |
| return ( | |
| fromType.name === EditorMenuTypes.OrderedList && | |
| toType.name === EditorMenuTypes.BulletList | |
| ); | |
| }; | |
| const fromBulletToOrderedList = (fromType: NodeType, toType: NodeType) => { | |
| return ( | |
| fromType.name === EditorMenuTypes.BulletList && | |
| toType.name === EditorMenuTypes.OrderedList | |
| ); | |
| }; | |
| const convertListAttributes = ( | |
| fromType: NodeType, | |
| toType: NodeType, | |
| attrs: Record<string, any>, | |
| ) => { | |
| const newAttrs = { ...attrs }; | |
| if (fromOrderedToBulletList(fromType, toType)) { | |
| // Bullet lists generally do not need an "order" attribute. | |
| delete newAttrs.order; | |
| } else if (fromBulletToOrderedList(fromType, toType)) { | |
| // For ordered lists, set a default start if not present. | |
| newAttrs.order = newAttrs.order || 1; | |
| } | |
| return newAttrs; | |
| }; | |
| const fromOrderedToBulletList = (fromType: NodeType, toType: NodeType) => { | |
| return ( | |
| fromType.name === EditorMenuTypes.OrderedList && | |
| toType.name === EditorMenuTypes.BulletList | |
| ); | |
| }; | |
| const fromBulletToOrderedList = (fromType: NodeType, toType: NodeType) => { | |
| return ( | |
| fromType.name === EditorMenuTypes.BulletList && | |
| toType.name === EditorMenuTypes.OrderedList | |
| ); | |
| }; | |
| const convertListAttributes = ( | |
| fromType: NodeType, | |
| toType: NodeType, | |
| attrs: Record<string, any>, | |
| ) => { | |
| const newAttrs = { ...attrs }; | |
| if (fromOrderedToBulletList(fromType, toType)) { | |
| // Bullet lists generally do not need an "order" attribute. | |
| newAttrs.order = undefined; | |
| } else if (fromBulletToOrderedList(fromType, toType)) { | |
| // For ordered lists, set a default start if not present. | |
| newAttrs.order = newAttrs.order || 1; | |
| } | |
| return newAttrs; | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 124-124: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
| */ | ||
| export function createDocWithHeading( | ||
| text: string, | ||
| level: number = 1, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove redundant type annotation.
The type annotation for the level parameter is redundant since it's already inferred from the default value.
- level: number = 1,
+ level = 1,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| level: number = 1, | |
| level = 1, |
🧰 Tools
🪛 Biome (1.9.4)
[error] 130-130: This type annotation is trivially inferred from its initialization.
Safe fix: Remove the type annotation.
(lint/style/noInferrableTypes)
| if (firstChild && firstChild.firstChild) { | ||
| const linkMark = firstChild.firstChild.marks.find( | ||
| (m) => m.type.name === 'link', | ||
| ); | ||
| expect(linkMark).toBeDefined(); | ||
|
|
||
| if (linkMark) { | ||
| expect(linkMark.attrs.href).toBe('https://example.com'); | ||
| expect(linkMark.attrs.title).toBe('Example'); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using optional chaining for safer property access.
Using optional chaining would make the code more robust by safely handling potential null or undefined values.
- if (firstChild && firstChild.firstChild) {
- const linkMark = firstChild.firstChild.marks.find(
+ const linkMark = firstChild?.firstChild?.marks.find(
(m) => m.type.name === 'link',
- );
- expect(linkMark).toBeDefined();
-
- if (linkMark) {
- expect(linkMark.attrs.href).toBe('https://example.com');
- expect(linkMark.attrs.title).toBe('Example');
- }
+ );
+ expect(linkMark).toBeDefined();
+
+ if (linkMark) {
+ expect(linkMark.attrs.href).toBe('https://example.com');
+ expect(linkMark.attrs.title).toBe('Example');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (firstChild && firstChild.firstChild) { | |
| const linkMark = firstChild.firstChild.marks.find( | |
| (m) => m.type.name === 'link', | |
| ); | |
| expect(linkMark).toBeDefined(); | |
| if (linkMark) { | |
| expect(linkMark.attrs.href).toBe('https://example.com'); | |
| expect(linkMark.attrs.title).toBe('Example'); | |
| } | |
| } | |
| const linkMark = firstChild?.firstChild?.marks.find( | |
| (m) => m.type.name === 'link', | |
| ); | |
| expect(linkMark).toBeDefined(); | |
| if (linkMark) { | |
| expect(linkMark.attrs.href).toBe('https://example.com'); | |
| expect(linkMark.attrs.title).toBe('Example'); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 119-119: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| delete global.window; | ||
| delete global.document; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Replace delete operator with undefined assignment for better performance.
Using the delete operator can have performance implications. Consider assigning undefined instead.
- delete global.window;
- delete global.document;
+ global.window = undefined;
+ global.document = undefined;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| delete global.window; | |
| delete global.document; | |
| global.window = undefined; | |
| global.document = undefined; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 210-210: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 211-211: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
This PR adds utility functions that use built in functions from prosemirror to handle operations related to creating/removing and toggling between list types in the text editor.
Tests are also added for this functionality.
Fixes: #3313
Summary by CodeRabbit
New Features
Documentation
Chores