Opened 11 months ago

Last modified 4 weeks ago

#5102 reopened defect

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 (7)

comment:1 Changed 6 months ago by juliandoucette

  • Milestone acceptableads.com/committee cleanup deleted

comment:2 Changed 6 months ago by juliandoucette

  • Owner set to juliandoucette

comment:3 Changed 6 months ago by juliandoucette

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

comment:4 Changed 4 months ago by juliandoucette

  • Status changed from reviewing to reopened

comment:5 Changed 5 weeks ago by juliandoucette

  • Owner juliandoucette deleted

comment:6 Changed 4 weeks ago by juliandoucette

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

comment:7 Changed 4 weeks ago 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;
}
Note: See TracTickets for help on using tickets.