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

Improve vertical alignment of phase banner tag on mobile devices #2132

Merged
merged 2 commits into from
Mar 2, 2021
Merged

Improve vertical alignment of phase banner tag on mobile devices #2132

merged 2 commits into from
Mar 2, 2021

Conversation

matthewmascord
Copy link
Contributor

This PR is designed to improve the vertical alignment of the phase banner tag when viewed on mobile devices. Following consultation with @timpaul and @christopherthomasdesign it was agreed that the desired behaviour should be:

  • single line of text = centre-aligned
  • multiple lines of text = top-aligned

In order to fix this problem, it was necessary to wrap the tag element in a SPAN with display: table-cell, allowing the vertical alignment of the tag to be controlled.

Before:
image

After:
image
image

Tested on:
Windows 7, IE8
Windows 7, IE9
Windows 7, IE10
Windows 10, IE11
Windows 10, Firefox 84
Windows 10, Chrome 88
Windows 10, Edge 88
Galaxy S8, Android 7, Chrome
Galaxy S8, Android 7, Samsung Internet
iPhone 8, iOS 13, Safari
iPhone 8, iOS 13, Chrome
macOS Catalina, Firefox 85
macOS Catalina, Safari 14
macOS Catalina, Chrome 88

@hannalaakso hannalaakso added the awaiting triage Needs triaging by team label Feb 1, 2021
@trang-erskine trang-erskine removed the awaiting triage Needs triaging by team label Feb 8, 2021
@matthewmascord
Copy link
Contributor Author

Hello, did we have any thoughts around this? Any concerns?

@timpaul
Copy link
Contributor

timpaul commented Feb 22, 2021

Hi @matthewmascord - sorry for the delay in getting back. We're really keen to avoid any additional markup changes if possible, as that would then be a breaking change. Do you think there's a way of achieving something similar without the additional span?

@matthewmascord
Copy link
Contributor Author

Thanks @timpaul , interesting question, I can certainly look into that. I didn't realise adding markup was always a breaking change - what makes it breaking exactly?

@matthewmascord
Copy link
Contributor Author

Hello @timpaul, I've simplified the solution removing the markup changes and retested on the above devices.

It turned out that the solution was backwards compatible for HTML users with no markup changes because the default behaviour without the additional span with display: table-cell caused the tag to be aligned to the top anyway. Given this, I've removed the markup change for simplicity.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2132 February 23, 2021 09:30 Inactive
@vanitabarrett vanitabarrett added this to Needs review 🔍 in Design System Sprint Board via automation Feb 26, 2021
Copy link
Contributor

@CharlotteDowns CharlotteDowns left a comment

Choose a reason for hiding this comment

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

@matthewmascord I am approving this suggestion, thank you for making it and testing it across browsers.

@CharlotteDowns CharlotteDowns merged commit 131c55a into alphagov:master Mar 2, 2021
Design System Sprint Board automation moved this from Needs review 🔍 to Done 🏁 Mar 2, 2021
CharlotteDowns added a commit that referenced this pull request Mar 3, 2021
This pull request improves vertical alignment of phase banner tag on mobile devices
CharlotteDowns added a commit that referenced this pull request Mar 4, 2021
@36degrees 36degrees added this to the [NEXT] milestone Mar 5, 2021
@36degrees 36degrees mentioned this pull request May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants