Opened on 04/04/2017 at 06:58:04 PM

Closed on 04/05/2018 at 12:57:06 PM

Last modified on 04/09/2018 at 04:48:35 PM

#5102 closed defect (duplicate)

hr under heading is semantically incorrect on acceptableads.com

Reported by:	juliandoucette	Assignee:
Priority:	P4	Milestone:
Module:	Websites	Keywords:	goodfirstbug
Cc:	saroyanm, ire	Blocked By:
Blocking:		Platform:	Unknown / Cross platform
Ready:	yes	Confidential:	no
Tester:	Unknown	Verified working:	no
Review URL(s):	old: https://codereview.adblockplus.org/29536654

Description

Environment

https://acceptableads.com/

How to reproduce

See narrow border above and below many headings.

Observed behaviour

Border is implemented using <hr>

Expected behaviour

Border should be implemented using CSS

Explanation

The <hr> element represents a paragraph-level thematic break.
-- w3 wiki

This narrow heading border is not meant to be a thematic break.

Change History (9)

comment:1 Changed on 09/01/2017 at 01:31:21 PM by juliandoucette

Milestone acceptableads.com/committee cleanup deleted

comment:2 Changed on 09/05/2017 at 12:54:12 PM by juliandoucette

Owner set to juliandoucette

comment:3 Changed on 09/05/2017 at 02:40:34 PM by juliandoucette

Review URL(s) modified (diff)
Status changed from new to reviewing

comment:4 Changed on 10/26/2017 at 03:09:20 PM by juliandoucette

Status changed from reviewing to reopened

comment:5 Changed on 01/16/2018 at 11:40:27 PM by juliandoucette

Owner juliandoucette deleted

comment:6 Changed on 01/25/2018 at 02:16:21 AM by juliandoucette

Cc ire added
Owner set to juliandoucette
Review URL(s) modified (diff)

comment:7 Changed on 01/25/2018 at 02:49:10 AM by juliandoucette

Owner juliandoucette deleted
Priority changed from P3 to P4

This is easy to fix, but:

There are many other minor issues attached that cannot be easily separated
This has no negative impact on our SEO or accessibility AFAICT

For that reason, I'm going to disown and de-prioritize this issue.

This may be a good first issue for a trainee. But I don't think we should spend our time on it unnecessarily.

Here is an example implementation (which assumes that we want to use both the hr element and a class applied to headings for a similar purpose):

hr,
.title::after
{
  height: 3px;
  width: 24px;
  border: none;
  background-color: $primary-fg;
}

hr 
{ 
  margin-top: $sm;
  margin-bottom: $sm
}

.center hr 
{
  margin-left: auto;
  margin-right: auto;
}

.title { position: relative; }

.title::after
{
  display: block;
  position: absolute;
  bottom: -35px;
  left: 0;
  content: "";

  @media (max-width: $mobile-breakpoint)
  {
    bottom: -16px;
  }
}

[dir="rtl"] .title::after
{
  left: auto;
  right: 0;
}

comment:8 Changed on 04/05/2018 at 12:57:06 PM by juliandoucette

Resolution set to duplicate
Status changed from reopened to closed

Migrated to https://gitlab.com/eyeo/websites/web.acceptableads.com/issues/7

comment:9 Changed on 04/09/2018 at 04:48:35 PM by abpbot

A commit referencing this issue has landed:
Issue 5102 - Refactored h1 border bottom from hr to CSS

Context Navigation