Issue 6061 - website-defaults unstyled ul doesn't unstyle li

The unstyled class was not previously removing the list style type of list items when applied to a ul. This change also enables us to apply unstyled to a parent element of a list.

[ire, 2017-11-27 08:40:02]

Thanks Julian.

It seems that the reason that only the selectors `.unstyled ul, ul.unstyled`
don't work for the list items is because `.content ul` is defined later. Since
this is a utility class, I suggest that we should either:

1. Move utility classes to be included last
2. Add an !important to the styles so they won't be overriden in this way

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.html
File pages/utilities.html (right):

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.htm...
pages/utilities.html:38: <h4 id="unstyled-list-parent"><pre>.unstylied</pre>
applied to parent</h4>
The <code> element is better for inline "code"

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.htm...
pages/utilities.html:38: <h4 id="unstyled-list-parent"><pre>.unstylied</pre>
applied to parent</h4>
Type: "unstylied"

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
File static/scss/_utilities.scss (right):

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
static/scss/_utilities.scss:120: .unstyled li,
I don't think this selector is needed because if .unstyled is applied to the
parent <ul>, it will be applied to the <li> as well. Also since .unstyled ul is
here.

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
static/scss/_utilities.scss:121: li.unstyled
NIT: I can't think of a case where only one list item will need to be unstyled

[juliandoucette, 2017-11-27 13:32:10]

Thanks Ire!

> It seems that the reason that only the selectors `.unstyled ul, ul.unstyled`
> don't work for the list items is because `.content ul` is defined later. Since
> this is a utility class, I suggest that we should either:
> 
> 1. Move utility classes to be included last
> 2. Add an !important to the styles so they won't be overriden in this way

I think that we should close this review unfinished and do both of these as a
separate issue. I think that !important is justified because a utility class is
(or "should be" - I think) very specific.

Do you agree?

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.html
File pages/utilities.html (right):

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.htm...
pages/utilities.html:38: <h4 id="unstyled-list-parent"><pre>.unstylied</pre>
applied to parent</h4>
On 2017/11/27 08:40:02, ire wrote:
> The <code> element is better for inline "code"

Good catch.

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.htm...
pages/utilities.html:38: <h4 id="unstyled-list-parent"><pre>.unstylied</pre>
applied to parent</h4>
On 2017/11/27 08:40:02, ire wrote:
> Type: "unstylied"

You mean typo?

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
File static/scss/_utilities.scss (right):

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
static/scss/_utilities.scss:120: .unstyled li,
On 2017/11/27 08:40:02, ire wrote:
> I don't think this selector is needed because if .unstyled is applied to the
> parent <ul>, it will be applied to the <li> as well. Also since .unstyled ul
is
> here.

Good catch.

https://codereview.adblockplus.org/29612659/diff/29612688/static/scss/_utilit...
static/scss/_utilities.scss:121: li.unstyled
On 2017/11/27 08:40:02, ire wrote:
> NIT: I can't think of a case where only one list item will need to be unstyled

Acknowledged.

[ire, 2017-11-27 18:07:56]

On 2017/11/27 13:32:10, juliandoucette wrote:
> Thanks Ire!
> 
> > It seems that the reason that only the selectors `.unstyled ul, ul.unstyled`
> > don't work for the list items is because `.content ul` is defined later.
Since
> > this is a utility class, I suggest that we should either:
> > 
> > 1. Move utility classes to be included last
> > 2. Add an !important to the styles so they won't be overriden in this way
> 
> I think that we should close this review unfinished and do both of these as a
> separate issue. I think that !important is justified because a utility class
is
> (or "should be" - I think) very specific.
> 
> Do you agree?

Yes I agree

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.html
File pages/utilities.html (right):

https://codereview.adblockplus.org/29612659/diff/29612688/pages/utilities.htm...
pages/utilities.html:38: <h4 id="unstyled-list-parent"><pre>.unstylied</pre>
applied to parent</h4>
On 2017/11/27 13:32:09, juliandoucette wrote:
> On 2017/11/27 08:40:02, ire wrote:
> > Type: "unstylied"
> 
> You mean typo?

Haha yes I did mean typo :P

[juliandoucette, 2017-12-01 15:23:44]

Closed.

I will add !importants in one Patchset and re-order main.scss in another.