1143797 - Allow clicking on suggested explanation text to see overlay explaining the suggested tile

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Ed Lee :Mardak]

Similar to bug 1138817, but instead of triggering from [suggested], the user clicks on the explanation label.

dcrobot has ideas of on hover styling to let the user know it's clickable, e.g., pointer cursor, outline/shadow of a raised button.

Comment 1

[Ed Lee :Mardak]

Text: This site is suggested to you by Firefox. You can remove it at any time by clicking the [X] button. Learn more…

So probably..

This site is suggested to you by Firefox. You can remove it at any time by clicking the %1$S button. %2$S

Comment 2

[Ed Lee :Mardak]

Attachment: 03 History Tiles MVP - Reference Rollover.png

Comment 3

[Ed Lee :Mardak]

Attachment: 04 History Tiles MVP - Reference Layover.png

Comment 4

[Ed Lee :Mardak]

Sorry, another string change:

"This site is suggested to you by Firefox. You can remove it at any time by clicking the %1$S button. %2$S"

to:

"This site is suggested to you by Mozilla. You can remove it at any time by clicking the %1$S button. %2$S"

Comment 5

[Marina Samuel [:emtwo]]

Attachment: v1: Add suggestion legal text explanation overlay on user text click

Note: I also made several relevant changes from 'related' -> 'suggested' since we're moving toward renaming all the variables (bug 1136208)

Comment 6

[Marina Samuel [:emtwo]]

Attachment: click_state.png

Comment 7

[Marina Samuel [:emtwo]]

Attachment: hover_state.png

Comment 8

[Drew Willcoxon :adw]

Comment on attachment 8580941 [details] [diff] [review]
v1: Add suggestion legal text explanation overlay on user text click

Review of attachment 8580941 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, Marina.

This is unrelated to your patch, but I notice that the X button in the legal text overlay isn't vertically centered with the text in its line.  Would you mind filing a bug to fix that?

::: browser/base/content/newtab/sites.js
@@ +242,5 @@
>        button.removeAttribute("active");
>      }
>      else {
>        let explain = document.createElementNS(HTML_NAMESPACE, "div");
> +      explain.className = explanationTextClass.slice(1);

Please add a brief comment that the slice is to remove the dot.

Comment 9

[Marina Samuel [:emtwo]]

Attachment: legal_misaligned.png

Drew, just confirming I understood correctly, do you mean the (X) should be vertically centered instead of having its bottom aligned with the text there? Thanks!

Comment 10

[Drew Willcoxon :adw]

Yeah, I think so anyway. :-)  I don't mean for that to block any of your ongoing work though.

Comment 11

[Marina Samuel [:emtwo]]

Oh, it's not blocking. I was just wondering because the bottom aligned seemed like what I would expect :) Either way, now it's filed!

Comment 12

[Marina Samuel [:emtwo]]

https://hg.mozilla.org/integration/fx-team/rev/e70798a79585

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e70798a79585

Comment 14

[Marina Samuel [:emtwo]]

Attachment: [Aurora-Uplift] Add suggestion legal text explanation overlay on user text click

Comment 15

[Cornel Ionce [:noni]]

Verified fixed on Windows 7 64-bit, Mac OS X 10.9.5, Windows 8.1 64-bit and Ubuntu 12.04 32-bit using:
- Firefox Aurora (build ID: 20150405004004);
- Latest Nightly (build ID: 20150405030238)

Comment 16

[Cornel Ionce [:noni]]

Also verified with Firefox 38 beta 2, build ID: 20150406174117 on Windows 7 64-bit, Mac OS X 10.9.5, and Ubuntu 14.04 32-bit.

Comment 17

[mcdavis941 (sporadically reading bugmail)]

It looks like this was the beta (Fx38) uplift patch for this bug: 

https://hg.mozilla.org/releases/mozilla-beta/rev/65f2aa5f2dd7