Skip to content
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

Implement editing part for List Styles feature #7801

Closed
pomek opened this issue Aug 7, 2020 · 6 comments · Fixed by #7861
Closed

Implement editing part for List Styles feature #7801

pomek opened this issue Aug 7, 2020 · 6 comments · Fixed by #7861
Assignees
Labels
package:list squad:core Issue to be handled by the Core team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@pomek
Copy link
Member

pomek commented Aug 7, 2020

Part of #1031.

@pomek pomek added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:list squad:core Issue to be handled by the Core team. labels Aug 7, 2020
@pomek pomek added this to the iteration 35 milestone Aug 7, 2020
@pomek pomek self-assigned this Aug 7, 2020
@pomek
Copy link
Member Author

pomek commented Aug 10, 2020

As for now, I have a basic implementation of the feature.

For a bulleted list, you can define a type of marker that will be used when rendering the list.

image

The same applies for numbered lists:

image

And even for nested and mixed lists:

image

However, it's the beginning of problems since the feature must be integrated with other plugins.

Indenting lists

When creating at some point a nested list, it should not inherit the list style attribute/value from the parent list.

■ List item 1.
■ List item 2.[]
■ List item 3.

editor.execute( 'indentList' );

■ List item 1.
    ○ List item 2.[]
■ List item 3.

However, the deeper structures should not be modified:

■ List item 1.
■ List item 2.[]
    ⁃ Nested item 1.
    ⁃ Nested item 2.
■ List item 3.

editor.execute( 'indentList' );

■ List item 1.
    ○ List item 2.[]
        ⁃ Nested item 1.
        ⁃ Nested item 2.
■ List item 3.

Also, it should work correctly when between list items, other nested lists are placed:

■ List item 1.
    ○ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
■ List item 3.[]
■ List item 4.

editor.execute( 'indentList' );

■ List item 1.
    ○ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
    ○ List item 3.[]
■ List item 4.

The attribute was restored to the default value. However, if List item 2 has modified the list style, List item 3 should copy the list style from 2.

■ List item 1.
    ♥ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
■ List item 3.[]
■ List item 4.

editor.execute( 'indentList' );

■ List item 1.
    ♥ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
    ♥ List item 3.[]
■ List item 4.

Outdenting lists

More or less it should revert changes made by the indentList feature.

A simple scenario:

■ List item 1.
    ○ List item 2.[]
■ List item 3.

editor.execute( 'outdentList' );

■ List item 1.
■ List item 2.[]
■ List item 3.

Don't modify deeper structures:

■ List item 1.
    ○ List item 2.[]
        ⁃ Nested item 1.
        ⁃ Nested item 2.
■ List item 3.

editor.execute( 'outdentList' );

■ List item 1.
■ List item 2.[]
    ⁃ Nested item 1.
    ⁃ Nested item 2.
■ List item 3.

And copy the value for the listStyle attribute from an item that is on the same (indent) level:

■ List item 1.
    ○ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
    ○ List item 3.[]
■ List item 4.

editor.execute( 'outdentList' );

■ List item 1.
    ○ List item 2.
        ⁃ Nested item 1.
        ⁃ Nested item 2.
■ List item 3.[]
■ List item 4.

However, the outdent feature can replace the listItem element with the paragraph element. No action is required in such a situation.

Merging a block to a list

A case:

Paragraph[]
■ List item 1.
■ List item 2.
■ List item 3.

editor.execute( 'bulletedList' )

The block item (paragraph) will be replaced with the listItem element. And the question is, should it copy the lists styles? In other words, should it be like:

* Paragraph[]
■ List item 1.
■ List item 2.
■ List item 3.

or

■ Paragraph[]
■ List item 1.
■ List item 2.
■ List item 3.

The later seems to be correct. Fortunately, the first item on the list cannot be nested, so it shouldn't be too hard to implement.

In all scenarios, I was considering operations between the same type of lists (all nested lists are bulleted or numbered, but never both at the same time). What in case of mixed lists? As for now, I am treating them as separate elements.

@pomek
Copy link
Member Author

pomek commented Aug 10, 2020

Another case:

image

What should happen after removing the paragraph? Should the list be merged and all rendered with circle marker or treat those as separate lists?

@pomek
Copy link
Member Author

pomek commented Aug 10, 2020

Another case:

image

After outdenting, items 2.1 and 2.2 should inherit list styles from the list item 3.

@pomek

This comment has been minimized.

@pomek
Copy link
Member Author

pomek commented Aug 15, 2020

Maybe listening to editor#deleteContent() with higher priority could help. We could detect whether the list will be merged and adjust the listStyle attribute for the merged (second) list.

@pomek
Copy link
Member Author

pomek commented Aug 19, 2020

All issues I've spotted are extracted into a separate issue: #7879

jodator added a commit that referenced this issue Aug 21, 2020
Feature (list): Introduced the list styles feature that allows customizing the list's marker of the list item elements. Closes #7801.

Feature (theme-lark): Creates styles for the `ListStylesUI` plugin (see #7803).

MINOR BREAKING CHANGE (ui): It is now possible to override existing components when [adding new ones](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_componentfactory-ComponentFactory.html#function-add) to the [component factory](https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_componentfactory-ComponentFactory.html) (previously an error was thrown) (see #7803).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:list squad:core Issue to be handled by the Core team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant