Olivero is opinionated about alignment of table th
https://git.drupalcode.org/project/olivero/-/blob/8.x-1.x/css/dist/compo...

Which means that if with CKEditor you want to text-align your table header centre - that does not work. CKEditor is correctly adding class="text-align-center" (see screenshot) - but the Olivero table th directive takes precedence.

I'm happy to put in a small patch that removes the table th from table.css - unless you can think of a better solution? Let me know!

Olivero table th

CommentFileSizeAuthor
#25 Screenshot 2020-07-20 at 4.50.55 PM.png384.69 KBkiran.kadam911
#24 3158372-24.patch1.12 KBkostyashupenko
#23 After patch - Role.png581.71 KBchetanbharambe
#23 After patch - Name1.png570.18 KBchetanbharambe
#23 After patch - email.png571.8 KBchetanbharambe
#23 After patch - Company name.png580.88 KBchetanbharambe
#19 table_alignment-3158372-19.patch1.24 KBsd9121
#11 th-alignment-3158372-11.patch1.05 KBkiran.kadam911
#11 Screenshot 2020-07-14 at 3.30.33 PM.png416.18 KBkiran.kadam911
#11 Screenshot 2020-07-14 at 3.31.15 PM.png474.47 KBkiran.kadam911
#8 Screen Shot 2020-07-13 at 7.44.26 AM.png156.78 KBkaring
#7 Screen Shot 2020-07-13 at 7.42.35 AM.png38.7 KBkaring
#7 Screen Shot 2020-07-13 at 7.42.35 AM.png38.7 KBkaring
#5 table.png169.62 KBsd9121
Screen Shot 2020-07-10 at 11.50.54 AM.png93.33 KBkaring

Comments

KarinG created an issue. See original summary.

karing’s picture

karing
Location 🇨🇦
commented
Issue summary: View changes
karing’s picture

karing
Location 🇨🇦
commented

Or probably a better solution: we can leave the default for table th as is and add this to ensure users have control with CKEditor:

[dir=ltr] table th.text-align-left {
  text-align: left;
}
[dir=ltr] table th.text-align-center {
  text-align: center;
}
[dir=ltr] table th.text-align-right {
  text-align: right;
}
sd9121’s picture

sd9121 commented
Assigned: Unassigned » sd9121
sd9121’s picture

sd9121 commented
Assigned: sd9121 » Unassigned
StatusFileSize
new table.png169.62 KB

Hi @KarinG,

When we add a table from CKEditor and add some alignment properties, that comes as an inline style.
Therefore, I don't think we need extra CSS.

Screenshot for reference:

table

Please add your suggestion on this.

Thanks!

karing’s picture

karing
Location 🇨🇦
commented
Assigned: Unassigned » karing

Thanks for your feedback! Let me re-reproduce this on a fresh site.

karing’s picture

karing
Location 🇨🇦
commented
StatusFileSize
new Screen Shot 2020-07-13 at 7.42.35 AM.png38.7 KB
new Screen Shot 2020-07-13 at 7.42.35 AM.png38.7 KB

I'm seeing a different inline style! :-)

Fresh site - git stash - git pull - CKEditor: configured alignment icons in the toolbar:

Basic Page -> create table with header -> highlighting all cells -> hitting centre align icon in the toolbar: cells align but th does not.
CKEditor Table

Olivero view showing inline style: text-align-center
Olivero Table

This is a D8.9.x site

karing’s picture

karing
Location 🇨🇦
commented
StatusFileSize
new Screen Shot 2020-07-13 at 7.44.26 AM.png156.78 KB
karing’s picture

karing
Location 🇨🇦
commented
Assigned: karing » Unassigned
Status: Active » Needs work
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: Unassigned » kiran.kadam911
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Assigned: kiran.kadam911 » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new Screenshot 2020-07-14 at 3.31.15 PM.png474.47 KB
new Screenshot 2020-07-14 at 3.30.33 PM.png416.18 KB
new th-alignment-3158372-11.patch1.05 KB

Kindly review the attached patch.

@KarinG As per class in TH css is added for alignment. Please check below SS,
Node edit:

Node view:

Thanks!

pratik_kamble’s picture

pratik_kamble
Location Pune, India
commented
Issue tags: +DIACWJuly2020
karing’s picture

karing
Location 🇨🇦
commented
Issue tags: -DIACWJuly2020 +Global2020

The proper tag for today is Global2020

karing’s picture

karing
Location 🇨🇦
commented

@kiran.kadam911 - sorry for the delay on getting back to you. Looking at your patch now. I think for there probably should be css to handle

text-align-left

as well?

karing’s picture

karing
Location 🇨🇦
commented
Status: Needs review » Needs work
pratik_kamble’s picture

pratik_kamble
Location Pune, India
commented
Issue tags: +DIACWJuly2020

@KarinG we have Contribution sprint tomorrow in India so I had tagged it for it.

karing’s picture

karing
Location 🇨🇦
commented

Ah I see! Great - sure can be both :-)

sd9121’s picture

sd9121 commented
Assigned: Unassigned » sd9121
sd9121’s picture

sd9121 commented
Assigned: sd9121 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new table_alignment-3158372-19.patch1.24 KB

Please review the patch.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented

Hello @KarinG,

I purposefully not added text-align-left because by default it's working as left alignment I checked the same on local. But if you think that is needed then we can go ahead with patch #19 provided by @sd9121.

Thanks!

deepalij’s picture

deepalij commented
Assigned: Unassigned » deepalij
chetanbharambe’s picture

chetanbharambe commented
Assigned: deepalij » chetanbharambe
chetanbharambe’s picture

chetanbharambe commented
Assigned: chetanbharambe » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new After patch - Company name.png580.88 KB
new After patch - email.png571.8 KB
new After patch - Name1.png570.18 KB
new After patch - Role.png581.71 KB

Patch #19 is applied successfully.

Testing Steps Followed:
Prerequisite: User should go to configuration -> click on text format and editors -> click on configure in front of Full HTML -> Drag and drop available left, right, center aligned buttons into the active toolbar format -> Save the configuration
- Create Article page
- Add 4 column table in ck editor
- set the respective column name as left, right, center
- Save it

Looks good to me.
Can be move to RTBC
Please refer attahced screenshots.

kostyashupenko’s picture

kostyashupenko
Primary language English
Location Omsk
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 3158372-24.patch1.12 KB
kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
StatusFileSize
new Screenshot 2020-07-20 at 4.50.55 PM.png384.69 KB 
diff --git a/css/src/components/table.css b/css/src/components/table.css
index c3260df..0615021 100644
--- a/css/src/components/table.css
+++ b/css/src/components/table.css
@@ -35,13 +35,12 @@ table {
   th {
     margin: 0;
-    text-align: left;
     letter-spacing: 0.05em;
     text-transform: uppercase;
     color: var(--color--gray-0);

@kostyashupenko Looking at your new patch you removed text-align: left which cause the default alignment of TH in the center if you not applied any alignment from ckeditor. About TD alignment by default it's left if you not applied any alignment from ckeditor. It's creating miss-alignment in between TH & TD if alignment is not selected from ckeditor alignment option. In my opinion we can go ahead with #19 or #11 & just need to update padding in it if required.

SS:

Feel free to add your opinion.

Thanks!

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Issue tags: +Bug Smash Initiative
karing’s picture

karing
Location 🇨🇦
commented

Reviewed at this morning's slack meeting:

+1 for #19

This issue is specific to letting users set alignments using CKEditor only - probably should not include any padding edits/changes.

kiran.kadam911’s picture

kiran.kadam911
he/him
Location Mumbai, India 🇮🇳
commented
Status: Needs review » Reviewed & tested by the community

Moving to RTBC, because patch #19 is tested by @chetanbharambe in #23

Thanks!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Reviewed & tested by the community » Fixed

#19 Looks good. Committing!

  • mherchel committed 639d115 on 8.x-1.x authored by sd9121 
    Issue #3158372 by kiran.kadam911, sd9121, kostyashupenko, KarinG,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.