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

Remove padding-right from last column in summary list row #2725

Merged
merged 3 commits into from
Jul 26, 2022

Conversation

querkmachine
Copy link
Member

Tiny change to CSS so that the last column in a summary list always has right padding removed, not only if the last column is an actions column. Resolves #1928.

The line that removes right padding from the actions column has been maintained for IE8, which doesn't support :last-child.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2725 July 22, 2022 12:49 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2725 July 22, 2022 12:50 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2725 July 22, 2022 12:53 Inactive
@querkmachine querkmachine marked this pull request as ready for review July 22, 2022 12:59
@querkmachine querkmachine requested a review from a team July 22, 2022 13:05
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Neat solution 👍🏻

One thing to be aware of is that when only some rows in a summary list have actions, there will only be padding on the 'value' column when there are actions.

(The rows that don't have actions use a pseudo-element to fill the void where the actions would be be, and TIL that pseudo-elements 'do not affect the interpretation of structural pseudo-classes'.)

I don't think this is necessarily worth worrying about, as there's no need for a 'gap' before the non-existent actions, but the text in the rows with and without actions will wrap at different points:

Screenshot 2022-07-22 at 14 07 49

@christopherthomasdesign or @Ciandelle thoughts? Any concerns with this approach?

@christopherthomasdesign
Copy link
Member

I think ideally all items in the same column would align with one another, but I don't think it's super critical here. I've seen summary cards used for large blocks of text but they're more commonly used for individual lines or lists, where you wouldn't really notice the difference. Even for blocks of text, the ragged right edge of it will obscure the lack of alignment a bit.

So unless there's a simple way to make something like Ollie's screenshot above line up, I'm pretty happy to go with this approach tbh

Tweaks CSS so that the last column always has right padding removed, not only if the last column is an actions column.
If some rows in the summary list had actions but others didn't, the rows without actions would result in a slightly wider 'last' column.

This resolves that issue by not applying the right padding removal to these rows.
@querkmachine
Copy link
Member Author

I've resolved the above alignment issue by adding a :not selector to exclude no-actions rows from removing the right padding.

.govuk-summary-list__row:not(.govuk-summary-list__row--no-actions) > :last-child {
    padding-right: 0;
}

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2725 July 26, 2022 12:26 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👏🏻 👍🏻

@querkmachine querkmachine merged commit aceb8d9 into main Jul 26, 2022
@querkmachine querkmachine deleted the kg-summary-list-last-padding branch July 26, 2022 12:41
@querkmachine querkmachine mentioned this pull request Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: remove right padding from last column in summary list
4 participants