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

Generate breadcrumb chevrons using pseudo-elements #407

Merged
merged 1 commit into from
Jan 4, 2018

Conversation

36degrees
Copy link
Contributor

This allows us to get rid of another two images, scales cleanly when zoomed, and ensures that the chevron is retained when users override the colours in their browser.

Chrome 63

screen shot 2018-01-02 at 16 18 43gmt

Firefox 57

screen shot 2018-01-02 at 16 11 08gmt

Firefox 57 with custom colours

screen shot 2018-01-02 at 16 09 52gmt

IE 8 (Windows 7)

win7_ie_8 0

IE 9 (Windows 7)

win7_ie_9 0

IE 10 (Windows 7)

win7_ie_10 0

IE 11 (Windows 7)

win7_ie_11 0

Edge (Windows 10)

win10_edge_15 0

Opera 12.16 (Windows 8.1)

win8 1_opera_12 16

Kindle Fire 2 (Android 4.0)

android_amazon-kindle-fire-2_4 0_portrait

Samsung Galaxy S5 (Android 4.4)

android_samsung-galaxy-s5_4 4_portrait

iPhone 5s (iOS 7.0)

ios_iphone-5s_7 0_portrait

iPhone 6 (iOS 8.3)

ios_iphone-6_8 3_portrait

https://trello.com/c/0I5XLLwU/520-generate-breadcrumb-chevrons-using-pseudo-elements

This allows us to get rid of another two images, scales cleanly when zoomed, and ensures that the chevron is retained when users override the colours in their browser.
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Generally looks good and great comments @36degrees 💯

However this didn't work for me on IE8 (Browserstack) on /components/breadcrumbs. This appears to be because the component view, along with a couple of others, uses layout-debug which doesn't include govuk-frontend-oldie.css for IE8. Not sure why the layout is different for those, this might have been a conversation I missed.

@36degrees
Copy link
Contributor Author

Yeah, I had to do some faffing about to get it to render correctly for IE8. I haven't committed that work because it wasn't a particularly good solution to the problem and it didn't feel like it should be part of this PR.

@hannalaakso
Copy link
Member

I actually think it looks okay on IE8 once the correct stylesheet is linked up (well what I can see on Browserstack anyway...). I don't think we should spend ages making things look perfect for IE8, functional is enough 🙂

@36degrees
Copy link
Contributor Author

Are you happy with this then @hannalaakso?

Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Yes looks great 🙂

@36degrees 36degrees merged commit 5a648ed into master Jan 4, 2018
@36degrees 36degrees deleted the breadcrumb-chevron-pseudo-element branch January 4, 2018 16:25
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.

3 participants