Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: static/scss/content/_typography.scss

Issue 29536654: Issue 5102 - Replaced semantically incorrect <hr> elements with :after styles (Closed) Base URL: https://hg.adblockplus.org/web.acceptableads.com
Patch Set: Created Sept. 5, 2017, 2:40 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: static/scss/content/_typography.scss
===================================================================
--- a/static/scss/content/_typography.scss
+++ b/static/scss/content/_typography.scss
@@ -411,39 +411,46 @@
pre
{
overflow: scroll;
margin: 0;
}
// Horizontal rules ////////////////////////////////////////////////////////////
-hr
+hr,
ire 2017/09/07 09:20:17 NIT: We aren't using the hr element anywhere else.
juliandoucette 2017/09/11 16:24:14 Acknowledged. I should have checked.
+.outer h1:after
{
+ display: block;
width: $sm + $xs;
height: 3px;
margin: $sm 0;
border: none;
- // hr background color is the line color
background-color: $primary-fg;
}
// Place hr under h1 or h2 inside hN margin
-h1 + hr,
-h2 + hr
+h2 + hr,
ire 2017/09/07 09:20:18 The h2 + hr selector isn't relevant anymore
juliandoucette 2017/09/11 16:24:14 Acknowledged. I should have checked.
+h1:after
ire 2017/09/07 09:20:18 This should be `.outer h1:after`. Also, all three
juliandoucette 2017/09/11 16:24:14 Acknowledged. If you are correct about hr and h2
{
+ content: "";
position: absolute;
margin: -$md 0 $md 0;
- @media (max-width: $mobile-breakpoint)
+ @media (max-width: $mobile-breakpoint)
{
margin-top: -$sm - 5px;
}
}
+.outer h1:after
+{
+ margin-top: 32px;
ire 2017/09/07 09:20:18 This style seems to be overriding the mobile style
juliandoucette 2017/09/11 16:24:14 Good catch.
+}
+
// center hr in centered blocks
.center hr
ire 2017/09/07 09:20:18 NIT: See comment above. I don't think this is rele
juliandoucette 2017/09/11 16:24:14 Acknowledged.
{
margin: $sm auto;
}
// Tables //////////////////////////////////////////////////////////////////////

Powered by Google App Engine
This is Rietveld