Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

H2 styling tweaks round 2 #412

Merged
merged 21 commits into from
Apr 11, 2019
Merged

H2 styling tweaks round 2 #412

merged 21 commits into from
Apr 11, 2019

Conversation

pjlee11
Copy link
Contributor

@pjlee11 pjlee11 commented Apr 2, 2019

Resolves #404
Follows up #406

Following the completion of #404 UX and dev sat together and reviewed the initial issue against the Zeplin design specs and found inconsistencies, this PR fixes those inconsistencies.

Overall change: Increases the padding-bottom of the H2 to be 24px and changes the media query to be 600px and above.

Code changes:

  • padding-bottom of the <h2> has been increased from 16px to 24px on all viewports
  • The breakpoints for the changes in the <h2> has been increased from the incorrect 400px to 600px

  • [ ] I have assigned myself to this PR and the corresponding issues
  • Tests added for new features
  • Test engineer approval

@pjlee11 pjlee11 self-assigned this Apr 2, 2019
@pjlee11 pjlee11 requested a review from a team as a code owner April 2, 2019 10:37
Co-Authored-By: pjlee11 <phil.lee.ext@bbc.co.uk>
@twinlensreflex
Copy link
Contributor

In firefox (quantum, 66.0.2 (64-bit)) I'm getting 0 padding-bottom and this when I inspect the element:
Screen Shot 2019-04-03 at 11 15 34

Co-Authored-By: pjlee11 <phil.lee.ext@bbc.co.uk>
@pjlee11
Copy link
Contributor Author

pjlee11 commented Apr 3, 2019

@eleanombre good spot, padding-bottom can't take two values ... very dumb typo from me 🤦‍♂️ I've updated ...

Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

The subheading has 0 top padding under 600px

dr3
dr3 previously approved these changes Apr 3, 2019
Copy link
Contributor

@dr3 dr3 left a comment

Choose a reason for hiding this comment

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

🤦

jk

twinlensreflex
twinlensreflex previously approved these changes Apr 3, 2019
Copy link
Contributor

@twinlensreflex twinlensreflex 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 👍

@pjlee11 pjlee11 dismissed stale reviews from j-pendlebury and dr3 via b28fd68 April 9, 2019 16:36
dr3
dr3 previously approved these changes Apr 9, 2019
DenisHdz
DenisHdz previously approved these changes Apr 9, 2019
Copy link
Contributor

@DenisHdz DenisHdz left a comment

Choose a reason for hiding this comment

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

👍🏻

sareh
sareh previously approved these changes Apr 9, 2019
@jamesbrumpton
Copy link
Contributor

LGTM. Happy for this to be merged.

@sareh sareh dismissed stale reviews from DenisHdz, dr3, and themself via 8eef9bb April 11, 2019 09:14
@jamesdonoh jamesdonoh merged commit 9e5fdea into latest Apr 11, 2019
@jamesdonoh jamesdonoh deleted the h2-styling-tweaks-round-2 branch April 11, 2019 10:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

h2 styling tweaks
9 participants