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

Separate configuration for styles in ol and ul list properties #15554

Closed
Witoso opened this issue Dec 18, 2023 · 6 comments · Fixed by #16614
Closed

Separate configuration for styles in ol and ul list properties #15554

Witoso opened this issue Dec 18, 2023 · 6 comments · Fixed by #16614
Assignees
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:list type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@Witoso
Copy link
Member

Witoso commented Dec 18, 2023

📝 Provide a description of the improvement

Add a possibility to configure the styles of list properties separately. For example, allow content editors to choose for list of styles for ol but not for ul's. Currently, the configuration affects both.

Requested by Drupal.

📃 Other details

  • Browser: …
  • OS: …
  • CKEditor version: …
  • Installed CKEditor plugins: …

If you'd like to see this improvement implemented, add a 👍 reaction to this post.

@Witoso Witoso added type:improvement This issue reports a possible enhancement of an existing feature. package:list domain:ui/ux This issue reports a problem related to UI or UX. labels Dec 18, 2023
@Witoso Witoso changed the title Separate configuration for ol and ul list properties Separate configuration for styles in ol and ul list properties Dec 27, 2023
@bkosborne
Copy link

Yes! This would be great. The "type" attribute on unordered lists is deprecated and we don't want our users to use it, but we do want them to use the attribute on ordered lists, where it has semantic meaning. This is one of the remaining blockers for us to upgrade ~1000 sites to CKEditor 5.

@wimleers
Copy link

wimleers commented Mar 18, 2024

One crucial part of information is missing from this issue: this is a hard blocker for Drupal to adopt the native <ol type> and <ul type> functionality over at https://www.drupal.org/project/drupal/issues/3274635

That issue has MANY followers; 46 right now. @bkosborne has a crazy number of sites blocked on this, but for context: Drupal 9/10 have hundreds of thousands of sites using them and the highest number of followers for a Drupal core issue that I've EVER seen is ~250. >100 followers gets you easily in the top 0.1% of issues. The count of 46 gets you in the top 1% probably, at minimum in the top 2%.

@Witoso This seems so bafflingly trivial to add … can you perhaps point to some prior art so that @bkosborne can contribute the changes himself?

@Witoso
Copy link
Member Author

Witoso commented Mar 18, 2024

While I understand the impact, I wouldn't judge this as trivial (maybe in LoCs). We need to create a non-breaking API change in one of the most used features. As of now, I don't have manpower to push this, but I will try to add it as a subissue to a current larger initiative.

AFAICS, it was ecosystem's decision to block this based on the non-separate config, as the type feature is provided by CKE5. Perhaps you could change the decision you made based on the feedback you got?

I'm also not sure where I could point, tactical cc to @arkflpc as I think you participated in the implementation.

@arkflpc
Copy link
Contributor

arkflpc commented Mar 18, 2024

It seems to be a quite small ticket, both in implementation and API design.

@Witoso
Copy link
Member Author

Witoso commented Mar 18, 2024

Ok, let's discuss it, and I will ping you @wimleers and @bkosborne.

@wimleers
Copy link

Great! Appreciate it :)

AFAICS, it was ecosystem's decision to block this based on the non-separate config, as the type feature is provided by CKE5. Perhaps you could change the decision you made based on the feedback you got?

This is not just "a decision" as if it's subjective and could've gone either way without consequences. Without this, it's an objective fact that we'd make some use cases impossible.

We need to create a non-breaking API change in one of the most used features.

Not breaking BC should be trivial:

{ list: { properties: { styles: true } } }

can remain supported, just also allow:

{ list: { properties: { styles: ['ol', 'ul'] } } }

(and make that the new internal representation, just map true to ['ol', 'ul'] … or one of many other approaches one could think of 😄)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:list type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants