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

Table alignment property is not working properly #6179

Closed
jodator opened this issue Feb 3, 2020 · 6 comments · Fixed by ckeditor/ckeditor5-table#234
Closed

Table alignment property is not working properly #6179

jodator opened this issue Feb 3, 2020 · 6 comments · Fixed by ckeditor/ckeditor5-table#234
Assignees
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.

Comments

@jodator
Copy link
Contributor

jodator commented Feb 3, 2020

📝 Provide detailed reproduction steps (if any)

Try to align the table on the latest master using table properties view.

✔️ Expected result

  • the table is visually set either to right or left or centered

❌ Actual result

  • table cells contents are aligned left/right

📃 Other details

There are a few problems:

  1. The alignment attribute on Table is picked up by Alignment plugin converters (:bug:)
  2. The downcast conversion of the alignment to style does not work with the editing styles (💅):
    • it could be set on the <figure> not on <table> (as now)
    • the <table> styles in the editing area could have less enforcing rules and could allow proposed inline styles.

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@jodator jodator added type:bug This issue reports a buggy (incorrect) behavior. package:table labels Feb 3, 2020
@jodator jodator added this to the iteration 29 milestone Feb 3, 2020
@jodator
Copy link
Contributor Author

jodator commented Feb 3, 2020

Also @oleq's insights on this in the PR: ckeditor/ckeditor5-table#232 (comment).

@Reinmar
Copy link
Member

Reinmar commented Feb 3, 2020

There are cases like lists, where we use listIndent, listType because a list item can be semantically renamed into a paragraph or a heading.

However, there are also cases like images, medias, tables, where such an item cannot be renamed. Those features are objects and their attributes will never end up on a different element.

Therefore, IMO, here we have a bug in the alignment package. It must be smarter convert attributes on elements which are not objects. And perhaps it might also check whether $text is allowed inside those elements to only handle leaf elements (skip e.g. blockQuote).

@jodator jodator self-assigned this Feb 4, 2020
@jodator
Copy link
Contributor Author

jodator commented Feb 5, 2020

Therefore, IMO, here we have a bug in the alignment package

Nope :) I've didn't test this feature manually because of no UI (which is fine). Also, this tickets prooves that we're able to iterate fast and find the bugs earlier and that this doesn't hurt us so much :)

The problem here was with priorities - if both converters have the same priority then the one added to the build earlier will win (or should win). Here, increasing the table's alignment priority helps - the other solution would be to change the attribute name.

What was caught here also is a lack of consuming in low-level attribute converter (using conversion.for().add()).

@jodator
Copy link
Contributor Author

jodator commented Feb 5, 2020

The "where to" convert is extracted to: #6197 as it needs some decisions.

@jerickcm
Copy link

jerickcm commented Sep 2, 2020

same problem , i am using it in nuxt the image can only be in center and right, cannot align it to the left.

ive used this for nuxt integration using

https://blowstack.com/blog/frameworks/create-ckeditor-5-custom-build

for developers who are experiencing the same problem

@DeltekDavid
Copy link

DeltekDavid commented May 23, 2024

Hello from 2024 :) Experienced this today using the nightly build. I can align the table center or right. Left-align does nothing. Tried both in code with defaultProperties and using the contextual Table Properties button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:table type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
4 participants