Skip to content
This repository has been archived by the owner. It is now read-only.

Add new feed and mail link design #50

Merged
merged 3 commits into from Feb 21, 2017
Merged

Add new feed and mail link design #50

merged 3 commits into from Feb 21, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Feb 16, 2017

https://trello.com/c/KtX7L1Md/514-travel-advice-publisher-multi-page-front-end-edit-email-alert-link-text-1

  • Optimise feed and mail icons
  • Clean up small screen view
  • Better align icons against text baseline
  • Update copy, users were clicking “email” thinking they could email
    someone
  • Don’t use h1 for subscriptions, fixes hierarchy
  • Remove title attributes from links now text is longer and
    self-explanatory

Wide viewport

Before After
screen shot 2017-02-16 at 16 43 07 screen shot 2017-02-16 at 16 42 50

Thin viewport

Before After
screen shot 2017-02-16 at 16 43 41 screen shot 2017-02-16 at 16 43 22
fofr added 2 commits Feb 16, 2017
5.1kb down to 500 bytes
* Clean up small screen view
* Better align icons against text baseline
* Update copy, users were clicking “email” thinking they could email
someone
* Don’t use h1 for subscriptions, fixes hierarchy
* Remove title attributes from links now text is longer and
self-explanatory
@fofr fofr requested a review from nickcolley Feb 16, 2017
@fofr fofr changed the title [Do not merge] Add new feed and mail link design Add new feed and mail link design Feb 17, 2017
@nickcolley
Copy link

@nickcolley nickcolley commented Feb 17, 2017

I think we can simplify the CSS to rely on the flow of the content rather than using a media query.

This would mean the content determines if it will stack, which does not happen on most viewports.

.subscriptions {
    @include bold-19;

    ul {
      list-style: none;
      margin-left: -$gutter-half / 2;
      margin-right: -$gutter-half / 2;
    }

    li {
      display: inline-block;
      margin-left: $gutter-half / 2;
      margin-right: $gutter-half / 2;
      margin-bottom: $gutter / 2;
    }

    a {
      text-decoration: none;
      padding-left: 28px;
      background-repeat: no-repeat;
      background-position: 0 20%;
      @include media(tablet) {
        background-position: 0 35%;
      }
    }
}

Then remove the padding-bottom from .page-navigation-container

Otherwise this is looking great 👍 , more accessible since we're removing the title 💥

@fofr
Copy link
Contributor Author

@fofr fofr commented Feb 21, 2017

@nickcolley Updated in 22362f2

* Remove need for media queries
* Let content wrap naturally and always be styled correctly
* Uses space better

Based on feedback from @nickcolley
@fofr fofr force-pushed the tweak-email-feed-links branch from 22362f2 to 470f962 Feb 21, 2017
@nickcolley
Copy link

@nickcolley nickcolley commented Feb 21, 2017

Okay I've tested this on IE8+, iOS and Android, looking good. 👍

@fofr fofr merged commit ec7c046 into master Feb 21, 2017
2 checks passed
2 checks passed
default Build #232 succeeded on Jenkins
Details
security/snyk No new vulnerabilities
Details
@fofr fofr deleted the tweak-email-feed-links branch Feb 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.