Skip to content

Add support for vertical alignment in Table cells#3787

Merged
sveltifier merged 15 commits intomainfrom
fix-table-cells-vertical-alignment-issue
Mar 18, 2026
Merged

Add support for vertical alignment in Table cells#3787
sveltifier merged 15 commits intomainfrom
fix-table-cells-vertical-alignment-issue

Conversation

@SriHV
Copy link
Copy Markdown
Contributor

@SriHV SriHV commented Feb 23, 2026

What is the context of this PR?

ONSDESYS-805

Added two new parameter valign which will set the vertical alignment of table cells to top, center or bottom

By default if these parameters are not provided, the alignment of the cells stays to the top.

How to review this PR

Check the new examples created and check vertical alignment changes when the parameter options are set to top, center or bottom

Checklist

This needs to be completed by the person raising the PR.

  • I have selected the correct Assignee
  • I have linked the correct Issue

@vercel
Copy link
Copy Markdown

vercel bot commented Feb 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
design-system Ready Ready Preview, Comment Mar 18, 2026 0:34am

@SriHV SriHV self-assigned this Feb 23, 2026
@SriHV SriHV added the Bug Something isn't working label Feb 23, 2026
@SriHV SriHV marked this pull request as ready for review February 23, 2026 12:47
@SriHV SriHV requested a review from a team as a code owner February 23, 2026 12:47
Copy link
Copy Markdown
Contributor

@Dino-ONS Dino-ONS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a suggestion... Maybe keep the vertical-align: top as the base default in the CSS and only override it when a parameter is present? Otherwise it may break existing clients that do not have a parameter setting and are using HTML instead. Also, there is no input validation and the alignment value is concatenated directly into the class name, so if someone passes an unexpected value, it will create an invalid class.

@SriHV
Copy link
Copy Markdown
Contributor Author

SriHV commented Mar 4, 2026

Just a suggestion... Maybe keep the vertical-align: top as the base default in the CSS and only override it when a parameter is present? Otherwise it may break existing clients that do not have a parameter setting and are using HTML instead. Also, there is no input validation and the alignment value is concatenated directly into the class name, so if someone passes an unexpected value, it will create an invalid class.

There seems to be a issue with the cms team when they are trying to change the vertical-align. To fix this, I've added vertical alignment in the modifier class and removed it from their base class

Copy link
Copy Markdown
Contributor

@sveltifier sveltifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The valign property is for td cells, but the docs and tests seem to be testing a top-level property?

ZatinoX

This comment was marked as duplicate.

…to fix-table-cells-vertical-alignment-issue
@sveltifier sveltifier changed the title Fix alignment issue of cells in table component Add support for vertical alignment in Table cells Mar 18, 2026
@sveltifier sveltifier removed the Bug Something isn't working label Mar 18, 2026
@sveltifier sveltifier added the Enhancement Change of existing feature label Mar 18, 2026
@sveltifier sveltifier merged commit be46646 into main Mar 18, 2026
12 checks passed
@sveltifier sveltifier deleted the fix-table-cells-vertical-alignment-issue branch March 18, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Change of existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants