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

Fix feedback component layout on mobile #1211

Merged
merged 3 commits into from
Nov 29, 2019

Conversation

kr8n3r
Copy link
Contributor

@kr8n3r kr8n3r commented Nov 28, 2019

What

Fix Feedback component layout on mobile.

  • uses CSS grid for mobile viewport. Grid is supported in at least two previous mobile browsers - Safari, Chrome, Firefox, Samsung internet that we a majority of users on
  • moves link classes to list elements and introduces additional classes to use with the grid positioning

Why

With #1207 we inadvertently broke the component's layout on mobile.

Before #1207

See: https://www.gov.uk/guidance/style-guide/a-to-z-of-gov-uk-style

After #1207

See: https://www.gov.uk/change-name-deed-poll

After this change

https://govuk-publishing-compo-pr-1211.herokuapp.com/component-guide/feedback

Alternative options

Trello ticket

@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 28, 2019 12:23 Inactive
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 28, 2019 12:46 Inactive
@kr8n3r kr8n3r changed the title WIP: Fix feedback component layout on mobile Fix feedback component layout on mobile Nov 28, 2019
@kr8n3r kr8n3r force-pushed the fix-feedback-component-layoput-on-mobile branch from 00d57d2 to 811d8e7 Compare November 28, 2019 13:24
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 28, 2019 13:24 Inactive
@kr8n3r kr8n3r marked this pull request as ready for review November 28, 2019 13:48
@kr8n3r kr8n3r force-pushed the fix-feedback-component-layoput-on-mobile branch from 811d8e7 to 28795f6 Compare November 29, 2019 10:16
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 29, 2019 10:16 Inactive
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

Our very first second CSS grid usage 🎉 ! Looks good. Some inconsistencies with IEx on very narrow screens that, as you mentioned, could be addressed by wrapping the code with feature queries.

@andysellick
Copy link
Contributor

@alex-ju
Copy link
Contributor

alex-ju commented Nov 29, 2019

@andysellick uuu.. I missed that one. Nice work @andysellick!

@kr8n3r kr8n3r force-pushed the fix-feedback-component-layoput-on-mobile branch from 28795f6 to a0128d8 Compare November 29, 2019 11:39
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 29, 2019 11:39 Inactive
With #1207 we inadvertently broke the component's layout on mobile.
This PR fixes the component layout on mobile by:

- using CSS grid for the mobile viewport
- moves link classes to list elements and introduces additional classes
to use with the grid positioning
@kr8n3r kr8n3r force-pushed the fix-feedback-component-layoput-on-mobile branch from a0128d8 to fb3d986 Compare November 29, 2019 11:42
@bevanloon bevanloon temporarily deployed to govuk-publishing-compo-pr-1211 November 29, 2019 11:43 Inactive
@kr8n3r
Copy link
Contributor Author

kr8n3r commented Nov 29, 2019

@alex-ju added fallback for non-grid supporting browsers
added @supports to each list item as I'm already reseting padding, otherwise I'd to

> li {
display:inline-block;
...
}

Screenshot 2019-11-29 at 11 43 17

@alex-ju
Copy link
Contributor

alex-ju commented Nov 29, 2019

@alex-ju added fallback for non-grid supporting browsers
added @supports to each list item as I'm already resetting padding

Super! Good to be merged.

@kr8n3r kr8n3r merged commit cac8001 into master Nov 29, 2019
@kr8n3r kr8n3r deleted the fix-feedback-component-layoput-on-mobile branch November 29, 2019 12:17
kr8n3r added a commit that referenced this pull request Nov 29, 2019
Includes

- Fix header environment label layout on mobile
[PR #1212](#1212)

- Fix Feedback component layout on mobile
[#1211](#1207)
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.

4 participants