Skip to content

Improve detecting which items should be reconverted when using skip-level lists#20104

Merged
mmotyczynska merged 5 commits into
cc/epic/9587-skip-level-listsfrom
cc/9765-changing-list-start-after-skip-level-not-visible
Apr 28, 2026
Merged

Improve detecting which items should be reconverted when using skip-level lists#20104
mmotyczynska merged 5 commits into
cc/epic/9587-skip-level-listsfrom
cc/9765-changing-list-start-after-skip-level-not-visible

Conversation

@mmotyczynska
Copy link
Copy Markdown
Contributor

🚀 Summary

A brief summary of what this PR changes.


📌 Related issues


💡 Additional information

Optional: Notes on decisions, edge cases, or anything helpful for reviewers.


🧾 Checklists

Use the following checklists to ensure important areas were not overlooked.
This does not apply to feature-branch merges.
If an item is not relevant to this type of change, simply leave it unchecked.

Author checklist

  • Is the changelog entry intentionally omitted?
  • Is the change backward-compatible?
  • Have you considered the impact on different editor setups and core interactions? (e.g., classic/inline/multi-root/many editors, typing, selection, paste, tables, lists, images, collaboration, pagination)
  • Has the change been manually verified in the relevant setups?
  • Does this change affect any of the above?
  • Is performance impacted?
  • Is accessibility affected?
  • Have tests been added that fail without this change (against regression)?
  • Have the API documentation, guides, feature digest, and related feature sections been updated where needed?
  • Have metadata files (ckeditor5-metadata.json) been updated if needed?
  • Are there any changes the team should be informed about (e.g. architectural, difficult to revert in future versions or having impact on other features)?
  • Were these changes documented (in Logbook)?

Reviewer checklist

  • PR description explains the changes and the chosen approach (especially when performance, API, or UX is affected).
  • The changelog entry is clear, user‑ or integrator-facing, and it describes any breaking changes.
  • All new external dependencies have been approved and mentioned in LICENSE.md (if any).
  • All human-readable, translateable strings in this PR been introduced using t() (if any).
  • I manually verified the change (e.g., in manual tests or documentation).
  • The target branch is correct.

Comment thread packages/ckeditor5-list/src/list/converters.ts Outdated
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0834c16. Configure here.

Comment thread packages/ckeditor5-list/src/list/converters.ts Outdated
// lower-indent ancestor (mirroring the downcast's fallback). Remember that item here
// so later we can compare the wrapper against the current model and tell whether it's
// still up to date.
for ( let i = itemIndent - 1; i >= 0; i-- ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could extract this loop to a local helper function? Sth like fillStackForIntermediates or sth like this.

Comment on lines +290 to +293
...Object.fromEntries(
Array.from( referenceItem.getAttributes() )
.filter( ( [ key ] ) => attributeNames.includes( key ) )
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is exactly the same as L256-259, so maybe a tiny local helper would make this code a bit more readable?

@mmotyczynska mmotyczynska merged commit c6b67cd into cc/epic/9587-skip-level-lists Apr 28, 2026
12 checks passed
@mmotyczynska mmotyczynska deleted the cc/9765-changing-list-start-after-skip-level-not-visible branch April 28, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants