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

Convert masthead logos to inline #1366

Merged
merged 92 commits into from May 14, 2021
Merged

Conversation

rmccar
Copy link
Contributor

@rmccar rmccar commented Feb 10, 2021

What is the context of this PR?

This PR converts the masthead logos to inline svgs. This allows us to set the focus colour of the logo in css.
Also fixes an issue with the focus state on the nisra logo being too small and adds the customHeaderLogo param to the docs.

How to review

  • Masthead logos all look as expected
  • Focusing on the logos adds the yellow highlighted background and logo colour is set to black
  • Focusing on the nisra variant is now big enough for the logo
  • customHeaderLogo is added to the docs

@rmccar rmccar marked this pull request as draft February 10, 2021 16:22
@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

Merging #1366 (5980ad1) into master (a27a3ef) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1366   +/-   ##
=======================================
  Coverage   93.59%   93.59%           
=======================================
  Files          33       33           
  Lines        1529     1529           
=======================================
  Hits         1431     1431           
  Misses         98       98           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a27a3ef...5980ad1. Read the comment docs.

@rmccar
Copy link
Contributor Author

rmccar commented Feb 10, 2021

@rmccar rmccar marked this pull request as ready for review February 11, 2021 16:20
@rmccar rmccar force-pushed the convert-masthead-logos-to-inline branch from 4c10392 to ad04bb2 Compare February 12, 2021 11:34
@jrbarnes9 jrbarnes9 added the Do not merge Don't merge this PR until this label is removed label Feb 12, 2021
@jrbarnes9
Copy link
Contributor

Footer icon missing
Screenshot 2021-02-15 at 11 25 45

@jrbarnes9
Copy link
Contributor

jrbarnes9 commented Feb 15, 2021

Can we get the focus states tighter around the logos?

Screenshot 2021-02-15 at 11 34 03
Screenshot 2021-02-15 at 11 33 31

@rmccar rmccar force-pushed the convert-masthead-logos-to-inline branch from e175ffe to 8c8a127 Compare April 30, 2021 14:54
@rmccar rmccar force-pushed the convert-masthead-logos-to-inline branch from 9e3d5fd to fe80482 Compare May 10, 2021 11:38
@rmccar rmccar requested a review from jrbarnes9 May 12, 2021 11:04
Copy link
Contributor

@jrbarnes9 jrbarnes9 left a comment

Choose a reason for hiding this comment

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

  • Need to move the paragraphs for 'Internal' and 'External with navigation' variants, above the examples

Comment on lines 28 to 30
The [ghost button](/components/button/#header-style) in the header moves to the [footer](/components/footer) when the page width is under the medium breakpoint (less than 740px).

The external header with language switch and save button, should be used across public facing services.
Copy link
Contributor

Choose a reason for hiding this comment

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

Flip the order of these paragraphs

src/components/header/index.njk Outdated Show resolved Hide resolved
rmccar and others added 2 commits May 13, 2021 15:13
Co-authored-by: Jon Barnes <43346934+jrbarnes9@users.noreply.github.com>
@rmccar rmccar requested a review from jrbarnes9 May 13, 2021 14:20
@rmccar rmccar merged commit 34d4f03 into master May 14, 2021
@rmccar rmccar deleted the convert-masthead-logos-to-inline branch May 14, 2021 11:02
boxadesign pushed a commit that referenced this pull request Feb 17, 2023
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.

None yet

4 participants