943701 - [Settings] the “Do Not Track” panel is broken

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Fabien Cazenave [:kaze]]

Attachment: screenshot — before patch

See attached screenshot:
 • the text is unreadable, even in English — we should have some line wrapping here;
 • the higlight area does not cover the whole list item.

Comment 1

[Fabien Cazenave [:kaze]]

Another regression of bug 914892.

Comment 2

[Fabien Cazenave [:kaze]]

Eitan, can you check this one too please?

Comment 3

[Fabien Cazenave [:kaze]]

The “description” class has been affected by bug 914892, please make sure to check all paragraphs in li.description elements.

Comment 4

[Eitan Isaacson [:eeejay]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14137

Pointer to Github pull-request

Comment 5

[Eitan Isaacson [:eeejay]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14137

Pointer to Github pull-request

Comment 6

[Fabien Cazenave [:kaze]]

Attachment: screenshot — after first patch

Much better, but the highlight does not cover the whole list item. Do you think you can fix that, too?

Comment 7

[Fabien Cazenave [:kaze]]

Comment on attachment 8339460 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14137

Please r? me again after having addressed my previous comment.

Comment 8

[Eitan Isaacson [:eeejay]]

Comment on attachment 8339460 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14137

After playing a lot more with the paddings/margins, I gave up. This fix, which I am not proud of, removes the :active styling from .description labels.

I also have a suspicion that no highlight here is perhaps a UX decision?

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Triage: severe UI breakage.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

eeejay, please de-assign yourself If you no longer working on this bug.

If you do, please set a target milestone. Thanks.

Comment 14

[Fabien Cazenave [:kaze]]

Comment on attachment 8339460 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/14137

I confess I’m not completely pleased by this patch, for the reasons Eitan mentioned (= all items that have multi-line descriptions will lose their :active styling), but I couldn’t come up with a better solution yet — and this patch sure is a necessary improvement.

Given the timeframe, I’d suggest to land this patch (hence my R+) and open another bug to fix the :active styling for the whole Settings app. Arthur, Evelyn, is that OK with you?

Comment 16

[Arthur Chen [:arthurcc][inactive after 6/30]]

The text was placed in a <p> before and has been move to a <span> inside a <label>. Hence it is applied with the following styles that I think is critical to this issue. 

ul li > label > span {
  line-height: 6rem;
}

ul li > label > * {
  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;
}

A proper fix would be adding a new semantic in which the above styles are overridden.

Comment 17

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Attachment: patch for Gaia/master

I try to unify almost all list items who looks like `descriptions` because we have a lot of variation of it

Comment 18

[Fabien Cazenave [:kaze]]

Comment on attachment 8349013 [details] [review]
patch for Gaia/master

Wow, nice work! Thanks for your quick help Pavel.

Eitan, does this look OK to you as well?

Comment 19

[Théo Chevalier]

Do you think it solves bug 942618 too?

Comment 20

[Eitan Isaacson [:eeejay]]

(In reply to Théo Chevalier [:tchevalier] from comment #19)
> Do you think it solves bug 942618 too?

Just tested that. And not quite. The checkbox is on the wrong side. But ultimately I think the fix should go in pavel's patch.

Comment 22

[Pavel Ivanov [:ivanovpavel][:pivanov] UX]

Thanks guys :)

Landed to master:
https://github.com/mozilla-b2g/gaia/commit/0e153b7211eb525d448470fdcabbd0f22663f9a0

Comment 23

[Fabien Cazenave [:kaze]]

You rock! Thanks for this really nice cleanup.

Comment 24

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 0e153b7211eb525d448470fdcabbd0f22663f9a0 to:
v1.3: 1f60e3f6a0f7a99eba75989eaabb1b1249c67766