Skip to content
This repository has been archived by the owner on Jul 10, 2020. It is now read-only.

Updates to hero graphic guidelines #472

Merged
merged 25 commits into from
Apr 6, 2017

Conversation

nataliafitzgerald
Copy link
Contributor

Short description explaining the high-level reason for the pull request

Additions

Removals

Changes

Testing

Review

Preview this PR without the whitespace changes

Screenshots

screencapture-nataliafitzgerald-github-io-design-manual-global-elements-heroes-html-1491492748971

Notes

Todos

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

@nataliafitzgerald
Copy link
Contributor Author

@jenn-franklin
Copy link
Member

Looks good! Thanks for your work on this.

Copy link
Contributor

@ajbush ajbush left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@sarahsimpson09
Copy link

👍

1 similar comment
@jordanafyne
Copy link

👍


Heroes should be the most prominent thing on the page, and help to establish visual hierarchy of where the page lives within the information architecture.
<img alt="Image of hero graphic" src="../static/img/hero/hero_use_case.png"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

We've preferred using absolute urls, which would update the source here to {{site.github.urll}}/static/img/hero/hero_use_case.png (We should update all the existing {{site.baseurl}} uses but that's outside the scope of this PR)

<div class="content-33">
![Image of hero with call to action at mobile size]({{ site.baseurl }}/static/img/hero/Hero_mobile.png)
<div class="content-50 content-last">
<p>Small screens (600-)</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation here should be two spaces.

@nataliafitzgerald nataliafitzgerald removed the request for review from Dnpizarro April 6, 2017 17:51
Copy link
Member

@ielerol ielerol left a comment

Choose a reason for hiding this comment

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

looks good! just a broken link to fix

<div class="content-33 content-last"></div>
<h2 id="desktop">Style at desktop size<span class="cf-code-link"><a href="https://github.com/cfpb/capital-framework/blob/master/src/cf-layout/src/cf-layout.less#L618-L620">View code <span class="cf-icon cf-icon-external-link"></span></a></span></h2>
<h4>When other options are better</h4>
- When introducing a specific piece of content, like a blog, press release, or other lengthy or detailed content, use the [Text introduction]({{site.baseurl}}/global-elements/text-introduction.html)
Copy link
Member

Choose a reason for hiding this comment

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

change the link to point to text-introductions.html

@nataliafitzgerald
Copy link
Contributor Author

nataliafitzgerald commented Apr 6, 2017

@ielerol - I fixed the broken link. Look good to go?

Copy link
Contributor

@kurzn kurzn left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

@Scotchester Scotchester left a comment

Choose a reason for hiding this comment

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

👍 Big improvement. Thanks!

@Scotchester Scotchester merged commit 07cc9fb into cfpb:gh-pages Apr 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants