Logo fixes #237

selfthinker · 2016-08-11T16:07:47Z

I initially only wanted to fix that the logo breaks layout with increased font size (#232).
But because I found a couple of small issue around the logo (invalid, never executed, non-standard or confusing code), I fixed and refactored a few more things. One of the changes includes changing the dimensions of the logo. The "new" logo (both versions) were done and provided to me by @markhurrell.

Three known issues with the new logo:

gov.uk_logotype_crown.png was not transparent before but is now. I don't know if that is intentional. I will ask Mark when he's back.
~~I have intentionally not changed the IE8-and-under version of the logo as it doesn't look broken. Please let me know if you think that is important to do.~~
The only reason why the logo needs some fiddly pixel-pushing is because the logo itself includes whitespace. I will ask Mark when he's back if we could change that. (I would probably better create a new PR for that case.)

I looked at the git history for most of my changes, please refer to the commit messages for further information (history and rationale).

Before/after screenshots

These screenshots were all made from Firefox on Mac.
The visible change is that the logo is grew 1px on both sides. Its bottom is still vertically aligned with the "G", so that it grows 1px to the top and 1px to the right. But the top space was removed again from an containing div, so the height dimensions of the left-hand elements should be exactly the same as before (except for the image itself), the width got pushed 1px to the right.

Screen logo before:

Screen logo after (hardly any changes, only the logo is bigger):

Print logo before:

Print logo after (a few more changes, for the better I think):

Browser tests

I tested in a lot of browsers via BrowerStack and it generally was fine. Some probably ignorable issues I found with this change:

iPhone 4 was not perfectly aligned, but iPhone 4S is
Kindle was not perfectly aligned either but that has other general issues as well
Galaxy S5 Mini (4.4.) dropped the "GOV.UK" below the logo, but Galaxy S4 (4.4) was fine again

What I didn't test: Because I don't know what else could potentially affect the header (other included elements or certain modifications) I didn't test any of that. It would be great if someone could let me know what else there is and how I can test it.

robinwhittleton · 2016-08-12T09:25:49Z

source/assets/stylesheets/_core-print.scss

-  font-weight: bold;
-  font-size: 24px;
-  margin-bottom: 1em;
+  margin: 3px 0 1em; // 3px addtional space at top for countering -3px re-poositioning of #logo img


‘addtional’ should be ‘additional’. ‘re-poositioning’ should be ‘re-positioning’

Oops, fixed.

robinwhittleton · 2016-08-12T09:28:10Z

Is there a reason why gov.uk_logotype_crown.png is using 1-bit transparency instead of translucent edges?

robinwhittleton · 2016-08-12T09:29:05Z

Also, did we consider moving to an SVG crown? Would be good to document why we decided not to here.

selfthinker · 2016-08-12T11:13:40Z

Regarding the transparency, I don't know. I can use graphic tools to a degree but felt not confident enough doing something with an image as important as the logo. That's why I asked Mark to make them. Let's ask him about the transparency when he's back.

We talked about using an SVG but if we wanted to do that, I think that should be in a separate PR. All of these commits here are fixing one thing or another. Using an SVG logo would be more of a feature than a bugfix.

markhurrell · 2016-08-22T14:21:09Z

hey both, used the 1bit transparency out of habit (was an old IE thing right?). it's on a black background so thought might as well put it on a black matte. happy to do it again with full transparency if you prefer.

being nit-picking, the GOV.UK looks like its sitting a tiny bit low on the new version (I guess this is because there's more whitespace around the top of the search bar now). could we shift it up a pixel or two?

robinwhittleton · 2016-08-22T14:22:40Z

IE6 and lower. I think we can do full translucency now :)

selfthinker · 2016-08-22T14:47:04Z

@markhurrell, sorry, I think I might have missed a pixel or so from the top when making the screenshot. Because nothing has changed on the dimensions at all, except what I described earlier:

The visible change is that the logo is grew 1px on both sides. Its bottom is still vertically aligned with the "G", so that it grows 1px to the top and 1px to the right. But the top space was removed again from an containing div, so the height dimensions of the left-hand elements should be exactly the same as before (except for the image itself), the width got pushed 1px to the right.

I validated that by making a screenshot (together with bits of the chrome) and measuring the pixels while zoomed in.

I guess it's probably best if we could look at it together on a real screen (and not a screenshot) to be sure. I can also push some pixels around at the same time if I need to. That should be the quickest to resolve any potential issues.

markhurrell · 2016-08-23T11:45:40Z

[full transparency PNG attached above... helpfully, is invisible against the white page]

selfthinker · 2016-08-23T16:07:11Z

@markhurrell, are you sure that is the correct image? I just put it into the folder and my git doesn't show me that there is any difference.

On another related note, I have made a new before/after gif which shows both versions after each other, together with the browser canvas on the side to make sure you can see the correct dimensions. (Sorry for the bad quality.)

alextea · 2016-08-23T16:13:36Z

@robinwhittleton the 1-bit transparency could be a side effect of using ImageAlpha incorrectly.

markhurrell · 2016-08-23T16:15:02Z

nope I did it deliberately because I care about ie6 users bruh

Mark Hurrell
Head of Design for GOV.UK
Government Digital Service

On 23 August 2016 at 17:13, Alex Torrance notifications@github.com wrote:

@robinwhittleton https://github.com/robinwhittleton the 1-bit
transparency could be a side effect of using ImageAlpha incorrectly.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#237 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABfCZRhRGq1ZQ7bxRuzfMZTJA_muglwPks5qixwygaJpZM4JiSJ0
.

selfthinker · 2016-08-23T16:34:05Z

I have updated the logo with the new one Mark provided. (Sorry, I didn't see any changes in git because I failed to copy the file correctly into the corresponding folder.) I also updated the one for IE8 and under (I just halved the new one).

NickColley · 2016-09-19T16:35:27Z

Looks good, only thing I've noticed is that on the print styles in Chrome the logo is not as expected.

Before

After

selfthinker · 2016-09-19T19:36:58Z

@NickColley, I believe this should fix that issue: alphagov/govuk_frontend_toolkit/pull/328

selfthinker · 2016-09-23T11:09:40Z

The other PR was merged, but I'm not sure how long it takes for the frontend toolkit to be packaged and released.
@NickColley, now that that last issue is fixed, would you rather a) wait with the merge of this PR until the other change is live, or b) merge this now anyway, or c) undo the offending two lines and then merge this and make the print change in another PR which can wait until the toolkit has been released?

NickColley · 2016-09-27T08:53:50Z

@selfthinker we should wait till upstream is up to date then update it here to fix the issue I think?

When something has a height (especially of px) it doesn't grow with the text (not when zooming but increasing just the font size). Just removing the `height` fixes it in this case.

* `line-height` should be unitless * Using padding on images is invalid, using margin instead

* Setting width and height via CSS is not necessary anymore since the dimensions were set in the HTML in c0e8bd3 * Removing the border on the logo img is not necessary anymore as it was removed on all images globally in db43128 * Setting `display` on the image is not necessary as it's also floating and everything that floats is always `display: block` * Setting a `line-height` has no effect on replaced elements like images * When the styling for the current print logo was changed in 0704a5e the old styling was not removed * Using `core-48` together with redefining the font-size and font-weight is (nearly) the same as using `bold-80`, the original PR alphagov/static#352 mentions that this was not intentional This was only changed to `bold-48`, not `bold-80` because of a bug in govuk_frontend_toolkit, see alphagov/govuk_frontend_toolkit#328

The logo was 71x62 and resized to be 35x31. As the ratio is slightly off because 71 is not divisible by 2 (there are no half pixels), this changes the logo to be 72x64. This also simplifies the CSS (removing unnecessary floats and spacing) and fixes vertical alignment of the logo in print.

selfthinker · 2016-09-29T19:11:51Z

@NickColley, as we investigated and found that govuk_template uses a 16 months old frontend toolkit and I don't think it's worth the hassle, I have removed the offending line and left a comment in the CSS accordingly.
We should definitely still try to update the dependency, but it should be separate from this PR.

I hope this can now be merged?

NickColley · 2016-10-06T17:14:56Z

LGTM 👍

GOV.UK crown logo image has been tweaked slightly, so our app didn’t match newer prototypes. Changes from here: alphagov/govuk_template#237

Makes the following changes: Remove generated gov.uk from relative print links alphagov/govuk_template#234 Fix extended footer on certain pages alphagov/govuk_template#177 Degrade gracefully when external JS can’t be loaded alphagov/govuk_template#248 Add docs for adding tabindex="-1" to fix the skiplink alphagov/govuk_template#250 Logo fixes alphagov/govuk_template#237 Remove external links styles alphagov/govuk_template#231 Don’t include both html5shiv and html5shiv-printshiv alphagov/govuk_template#254 Update govuk_frontend_toolkit to 5.0.0 alphagov/govuk_template#256 Fixed scala compilation failure for play template alphagov/govuk_template#261

selfthinker force-pushed the logo-fixes branch from d0caa0f to 3c37068 Compare August 11, 2016 18:16

robinwhittleton reviewed Aug 12, 2016
View reviewed changes

selfthinker force-pushed the logo-fixes branch from 3c37068 to 5497e31 Compare August 12, 2016 11:17

selfthinker force-pushed the logo-fixes branch from 5497e31 to 2ae521f Compare August 23, 2016 16:29

selfthinker force-pushed the logo-fixes branch from 2ae521f to aaf9930 Compare September 19, 2016 15:31

selfthinker mentioned this pull request Sep 19, 2016

Remove @media width query from print styles alphagov/govuk_frontend_toolkit#328

Merged

NickColley mentioned this pull request Sep 27, 2016

Bump version to 4.18.2 alphagov/govuk_frontend_toolkit#333

Merged

selfthinker added 4 commits September 29, 2016 15:08

Fix logo breaking layout when increasing font size

79b197b

When something has a height (especially of px) it doesn't grow with the text (not when zooming but increasing just the font size). Just removing the `height` fixes it in this case.

Fix invalid or non-standard CSS around logo

81108eb

* `line-height` should be unitless * Using padding on images is invalid, using margin instead

selfthinker force-pushed the logo-fixes branch from aaf9930 to 973c9e4 Compare September 29, 2016 19:08

NickColley merged commit 2876583 into master Oct 6, 2016

NickColley deleted the logo-fixes branch October 6, 2016 17:16

NickColley mentioned this pull request Oct 6, 2016

Logo breaks layout with increased font size #232

Closed

selfthinker mentioned this pull request Oct 6, 2016

Version 0.18.3 #253

Merged

quis added a commit to alphagov/notifications-admin that referenced this pull request Nov 18, 2016

Upgrade GOV.UK template

c4bdd9c

GOV.UK crown logo image has been tweaked slightly, so our app didn’t match newer prototypes. Changes from here: alphagov/govuk_template#237

Guntrisoft mentioned this pull request Dec 2, 2016

Upgrade govuk_template to version 0.19.1 guidance-guarantee-programme/pension_guidance#699

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logo fixes #237

Logo fixes #237

selfthinker commented Aug 11, 2016 •

edited

Loading

robinwhittleton Aug 12, 2016

selfthinker Aug 12, 2016

robinwhittleton commented Aug 12, 2016

robinwhittleton commented Aug 12, 2016

selfthinker commented Aug 12, 2016

markhurrell commented Aug 22, 2016

robinwhittleton commented Aug 22, 2016

selfthinker commented Aug 22, 2016

markhurrell commented Aug 23, 2016 •

edited

Loading

selfthinker commented Aug 23, 2016

alextea commented Aug 23, 2016

markhurrell commented Aug 23, 2016

selfthinker commented Aug 23, 2016

NickColley commented Sep 19, 2016

selfthinker commented Sep 19, 2016

selfthinker commented Sep 23, 2016

NickColley commented Sep 27, 2016

selfthinker commented Sep 29, 2016 •

edited

Loading

NickColley commented Oct 6, 2016

Logo fixes #237

Logo fixes #237

Conversation

selfthinker commented Aug 11, 2016 • edited Loading

Before/after screenshots

Browser tests

robinwhittleton Aug 12, 2016

Choose a reason for hiding this comment

selfthinker Aug 12, 2016

Choose a reason for hiding this comment

robinwhittleton commented Aug 12, 2016

robinwhittleton commented Aug 12, 2016

selfthinker commented Aug 12, 2016

markhurrell commented Aug 22, 2016

robinwhittleton commented Aug 22, 2016

selfthinker commented Aug 22, 2016

markhurrell commented Aug 23, 2016 • edited Loading

selfthinker commented Aug 23, 2016

alextea commented Aug 23, 2016

markhurrell commented Aug 23, 2016

selfthinker commented Aug 23, 2016

NickColley commented Sep 19, 2016

selfthinker commented Sep 19, 2016

selfthinker commented Sep 23, 2016

NickColley commented Sep 27, 2016

selfthinker commented Sep 29, 2016 • edited Loading

NickColley commented Oct 6, 2016

selfthinker commented Aug 11, 2016 •

edited

Loading

markhurrell commented Aug 23, 2016 •

edited

Loading

selfthinker commented Sep 29, 2016 •

edited

Loading