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

Summary Component - vertical alignment #663

Closed
SatnamSinghUK opened this issue Dec 2, 2020 · 4 comments · Fixed by #665
Closed

Summary Component - vertical alignment #663

SatnamSinghUK opened this issue Dec 2, 2020 · 4 comments · Fixed by #665
Milestone

Comments

@SatnamSinghUK
Copy link

Hi, I have an issue with the Summary Component.

All text aligns to the top fine.

But adding an image/map forces the Title and Change link to align to the bottom

What

Briefly describe the thing you’re proposing. (It could be a style, component or pattern or something to add to the content style guide.)

Why

Here a repro of the issue
https://jsfiddle.net/qc2tayw6/

I think this is a viable issue, if in a scenario where an image or a map or similar needs to be added, all the elements should align correctly.

Anything else

A proposed fix is adding something like this:

.nhsuk-summary-list__key,
.nhsuk-summary-list__value,
.nhsuk-summary-list__actions {
vertical-align: top;
}

So all elements align vertically to the top

@chrimesdev
Copy link
Member

chrimesdev commented Dec 2, 2020

Hey @SatnamSinghUK thanks for opening this issue.

This seems like a viable fix to the issue, do you want to open a pull request on the NHS.UK frontend repository with your proposed changes? The team can then review the changes and take this forward.

If you're not sure where to start we have the NHS.UK frontend contributing guidelines.

Thanks

@chrimesdev chrimesdev transferred this issue from nhsuk/nhsuk-service-manual-community-backlog Dec 2, 2020
@chrimesdev
Copy link
Member

I have transferred this issue to the NHS.UK frontend repository as it relates to an issue with code in the library.

@SatnamSinghUK
Copy link
Author

Hey Adam,

Thanks for the quick response on this.

Fantastic I will push the fix on that repo 👍

@chrimesdev
Copy link
Member

Thanks for that @SatnamSinghUK I will take a look tomorrow and get it merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants