Opened 2 years ago

Closed 18 months ago

#5327 closed defect (incomplete)

Refactor use of %headings selector on acceptableads.com

Reported by: ire Assignee:
Priority: P4 Milestone:
Module: Websites Keywords: goodfirstbug
Cc: juliandoucette, saroyanm, wspee, ire Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Background

In the acceptableads.com SCSS, there is a a %headings placeholder selector, which targets all headings:

h1, h2, h3, h4, h5, h6 .h1, .h2, .h3, .h4, .h5, .h6
{
  @extend %headings !optional;
}

This is used in several places to target specific headings. For example, this selector which targets a header within a card:

.card %headings
{
  @extend h3;
}

This results is unnecessarily long selectors when the CSS is generated.

What to change

Refactor all uses of the %headings selector, where a class can be applied instead. For example:

.card .card-heading
{
  @extend h3;
}

Change History (5)

comment:1 Changed 2 years ago by juliandoucette

  • Review URL(s) modified (diff)

Relevant @extend gotchas:

Also, IIRC there is no concept of @extend in CSS3.

comment:2 Changed 2 years ago by juliandoucette

Therefore, I think it's better to define component heading styles individually instead of extending default heading styles.

comment:3 Changed 2 years ago by juliandoucette

  • Keywords goodfirstbug added
  • Priority changed from Unknown to P4
  • Review URL(s) modified (diff)

comment:4 Changed 2 years ago by ire

  • Cc ire added

comment:5 Changed 18 months ago by juliandoucette

  • Resolution set to incomplete
  • Status changed from new to closed

I don't think that it's necessary to migrate this issue. We may re-address this or similar in a larger refactoring or redesign effort in the future if necessary.

Note: See TracTickets for help on using tickets.