Add support for vertical alignment in Table cells#3787
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dino-ONS
left a comment
There was a problem hiding this comment.
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 |
sveltifier
left a comment
There was a problem hiding this comment.
The valign property is for td cells, but the docs and tests seem to be testing a top-level property?
…thub.com/ONSdigital/design-system into fix-table-cells-vertical-alignment-issue
…to fix-table-cells-vertical-alignment-issue
What is the context of this PR?
ONSDESYS-805
Added two new parameter
valignwhich will set the vertical alignment of table cells to top, center or bottomBy 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.