paxuclus / list-style Goto Github PK

I looked into enabling the options for ordered lists, but I came across some things I don't want to touch before I am sure about the intentions behind them.

• list-style-type attribute
  This plugin uses an additional attribute on the list. This results in invalid HTML. Rather it should apply e.g. a data-list-style attribute.

• list-style as attribute
  I assume this plugin uses an additional attribute rather than the CSS property (as done by the ckeditor module) to allow for more flexibility with custom properties that are not valid CSS?

• class names
  This plugin describes list-type presets with both title and value – value being

  the value of the list-style-type attribute of the list.

  That however is not the case. Instead the id/key of the preset is used. In fact as far as I can tell, that value is used to upcast an existing class name into the preset ID (and is then lost from the element). That's a bit confusing I think. Is this deliberate? I would have expected the preset value in the view.

• breaking lists
  The list editing was pretty buggy for me, unfortunately, as the lists got split quite frequently even when selecting all content. In some parts, that I see connected to this issue, the plugin differs from the "original" ckeditor module. Is there a reason to this (i.e. to deliberately customize the "default" behavior)? I haven't checked, whether the ckeditor module has changed since the last changes here.

Apart from that there was a minor bug with the list-style menu staying open if I leave the element, while the button to close the menu is gone. This could be quickfixed with something like that on the button:

isOpen() {
	return this.shouldDisplayAdditionalButtons() && this.state.isOpen;
}

I tried around a bit with

editorOptions:
  formatting:
    listStyle: boolean|null|{ol?: boolean|null|string[]|Record<string, boolean|null>, ul?: ...}

but I think this is not very readable. I think I would prefer a separate editor option:

editorOptions:
  listStyle:
    ol: boolean|null|string[]|Record<string, boolean|null>
    ul: boolean|null|string[]|Record<string, boolean|null>

With

• true all available values
• false|null none of these (only default? maybe no menu if only default is available, but maybe it should be possible to select the list and revert to default..)
• string[] a list of enabled keys from the Lala.ListStyle.styles.{listType} setting
• Record<string, boolean|null> a dictionary of keys from the setting and whether they should be enabled or not

Is there a preferred configuration?

Since #6, the list styles are stored in a new attribute (data-list-style-type). Currently, the plugin is able to work with the old (list-style-type) and the new attribute names. To clean up the js and css, there should be a migration that changes all existing nodes to use the new attribute name.

TODO:

• Create migration (CR YAML vs. Doctrine SQL?) for nodes
• Remove handling for both attributes in js: https://github.com/nlx-lars/list-style/blob/master/Resources/Private/src/liststyle/liststyleediting.js#L88
• Remove handling for both attributes in css: https://github.com/nlx-lars/list-style/blob/master/Resources/Public/ListStyles.css#L1

In theory this plugin should work with newer versions, so only the composer requirements have to be adjusted.

TODO:

• trigger on push to main(?)
• yarn install && yarn build
• commit assets in Resources/Public to main