When an Olivero table has a row header, the alignment of row header text is awkward and last row border is confusing

Gauravvvv → made their first commit to this issue’s fork.

Please mention steps to reproduce, currently, it is not clear where we can find them.

I have updated the IS and added STR but making the first row and first column both a header is not clear for me. So I am not removing the STR tag.

Pushed the code for the alignment issue in table.

Thanks

Creating a view to reproduce this is super complicated. I merely created a table in ckeditor. Added the resulting markup for easier testing into the issue summary.

Updating the status needs review as nothing is left to do in this.

Applied the patch and last row border is removed now, adding screenshots below.

LGTM, applied patch for removing padding which is causing issue for alignment and it is resolved now. Please refer screenshots

Hi
Verified and tested MR !6566 on Drupal 11 for the Olivero theme
Applied patch successfully...

Testing Steps

1. Install drupal version
2. Install the Olivero theme and set it as the default
3. Add table and check for last row border and row alignment
4. Apply the patch and check for the same

Test Result:
The last row border is removed and the row alignment issue is fixed

Attaching screenshot for reference
RTBC+1

The changes visually look good, thanks! I have no idea if this is the best way to implement them in Olivero's CSS :)

Updated MR to use logical properties, can review.

As a UI change the proposed solution should have before/after screenshots in the issue summary (where its says TBD)

Added before and after patch screenshots. Changes looks good to me. Border bottom is removed, also alignment is fixed. Moving to RTBC.

Have a question about the vertical align change. It's pretty overkill. I think we should adjust the fontsize/padding of th in the table body instead. Like this we'll mess up layout for tables with cells that are not the same length and it could make them very hard to read.

Addressed feedback of #33, updated MR for same.

Hi
I've tested the Updated MR- MR !6566 on Drupal version 11.x
The patch applied successfully...

Test Result:
Font size and padding of the table heading are updated and visually also looks good

attaching screenshots for reference

RTBC+1

Keeping in "Needs review" for code verification and more reviews

need some validation from olivero maintainers for the size change.

Need to update the screenshots in the issue summary as well.

Updated as per @nod_ to use px instead of rem. Rest can be reviewed by Olivero maintainers as well.

Before pinging the sub-maintainer moving to NW for the issue summary update

Updated the screenshots in the IS

Thanks

Here, I have checked MR !6566 and applied it successfully. It has cleanly removed the last row border from the table I created and the font size remains in pixels after applying the MR from @karanpagare. I'm attaching the screenshot for review

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@Gauravvvv there was already a 11.x branch can you update that one please.

Binoli Lalani → made their first commit to this issue’s fork.

Hello,

I resolved the conflicts of 6566 MR and hide 7718 MR.

Please review.

Thank you!

Binoli Lalani → changed the visibility of the branch 3420865-Olivero-table-head to hidden.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

ahsannazir → made their first commit to this issue’s fork.