The Heartbeat notification bar requires some small visual changes. Original spec: https://bug1092376.bugzilla.mozilla.org/attachment.cgi?id=8526074 Current implementation: https://people.mozilla.org/~glind/all/Screen%20Shot%202015-02-26%20at%203.15.19%20PM.png There are a few changes needed to match the implementation to spec. 1: Heartbeat icon instead of the product icon 2: Font-size and font colour 3: Bar height 4: Spacing between stars 5: The close X

(Alessio, if you start this, just WIP it, else I will get to it tomorrow morning. Should ONLY involve chagnes to http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/UITour.inc.css#151)

Spec: http://people.mozilla.org/~mmaslaney/prompt/Prompt-spec.png Height: 40px; box-shadow: 0px, 1px, 0px, 0px rgba(0,0,0,0.35);

The spacing between the stars is 4px. For the closing X, can we repurpose the Tab Close asset?

This should address all the reported issues except the close button one.

(In reply to Alessio Placitelli [:Dexter] from comment #4) > Created attachment 8570537 [details] [diff] [review] > bug1137481.patch - WIP > > This should address all the reported issues except the close button one. Could you post a screenshot, :Dexter?

There you go, :sevaan!

This fixes: - all fonts inherit SIZE and FAMILY from system. (No need to specify) - border-bottom, because boxShadow (outset) doesn't work for NotificationBox - messageText.flex = 0 ; to collaps the space before the stars. (unclear how to add space back in!) - smaller stars (16px) and heartbeat (24) icon, per UX request. - "revert" close button style. To be done: The spacing should be something like: Y X [heart] X [message] X [stars] for Y = 6, X = 8 (maybe?). This needs some experimentation.

This patch includes the changes from Greg and adds the spacing between the heart, the message and the stars. 14px (6px + 8px) [heart] 8px [message] 8px [stars]

(The older Heartbeat Linux PNG is no longer relevant)

Updated heartbet UI on Linux.

Layout here is 4px [24 icon] 4px [trimmed text] 4px [stars]. Tell me what if any should change. 4px left of the icon is 'stock'.

(creates the screen shot. bases on alessios earlier patch.)

(Alessio, I suspect that spacing will be too big. Let mmaslaney look at the pic :) Right now in my final it's 4 [24] 4 [text] 4 [stars] [infinite] [x]. This will also need an uplift bug, to everywhere that 1111027 went.

OSX per alessio's most recent patch.

New topic (at the risk of causing heart attacks).. Should we consider a 'learn more' or '?' button over near the 'x' to link to more info? (per Cameron McCormack and David Miller in yammer). If so, needs design. Worth it now or delay?

Is there a page ready for a "Learn more" to be linked to?

Comment on attachment 8571477 [details] Screen Shot 2015-03-02 at 12.38.43 PM.png A little more space between the copy and the stars.

Could we change from "Please rate Firefox" to "Give Mozilla your rating of Firefox" ? I think this would make it a lot more clear what the purpose is and also take advantage of how wide this notification bar is.

( :jaws, perhaps in the future, wording will be revisited. This language was tested for response rate in several previous experiments, and performed best. Noted.)

(question text as: https://github.com/mozilla/self-repair-server/issues/88)

(In reply to Sevaan Franks [:sevaan] from comment #17) > Is there a page ready for a "Learn more" to be linked to? Hey Sevaan. We created a public wiki page for Heartbeat to link to from the release notes. Check it here: https://wiki.mozilla.org/Advocacy/heartbeat That is where the "Tell me more" or "What is this?" link would take the user. Since Heartbeat is persistent across tabs they would see that it is indeed from Mozilla and still have the opportunity to give us a score.

## General Problem To Be Solved (or Not!) - unclear who is asking is for the rating and what purpose. ## Some paths of solution 1. educational compaigns 2. decide it's not a problem for most people, and do the natural experiment. 3. change the question. 4. link to help / more info / messaging authority in UI element. 5. Wait a bit and think more (not beta 2 uplift, let it sit a week or two). (I favor 4: Link/Authority!) ## Ideas for Option 4: Linking To Help / Authority. ### disclaimer: these ideas are incomplete and intended to spark discussion! 1. Do nothing, close the bug, force lift Good: - easy (lazy!) - makes it "someone else's problem" Bad: - deal with complaints that people don't know what this is, and what it's doing. - authority confusion. 2. "Old style" Nb Button with text. "more info" Good: - mostly fast to impelement - already Bad: - ugly, disimilar styling - buttons close the bar by default - needs localization (string!), for TEXT and TOOLTIP unless we reuse an existing translation 3. Reuse other existing help buttons (like from #about:preferences) Good - looks nice, similar to 'close button' Bad - needs localization (string!) for TOOLTIP, unless we reuse an existing translation 4. Some other "this is a Message from Mozilla" indications. (Icons, 'small text')

I'm all for 4 as well, as long as we have UX approval :)

Please add a "Learn more." after the copy. The hyperlink colour should be #0095DD.

This patch modifies Heartbeat as follows: - Adds learn more link; - Adds the same spacing before and after the heart icon; - Adds more spacing after the learn more link; I've obsoleted all the other screenshots and patches as this was getting a bit confusing (at least for me).

That's the heartbeat UI v3 on Windows. @sevaan, does it look ok to you?

Also see link colors

Also see link colors at (http://cl.ly/image/0B273R1M1d3w) link: #0095DD Active: #008ACB hover #006B9D

Adds the missing "learn more" notification and fixes styles on OSx.

Comment on attachment 8573168 [details] Heartbeat-v3-Windows.PNG The "Please Rate Firefox" text seems to be a pixel higher than the Learn more. Also, why are we not doing this as a sentence? "Please rate Firefox. Learn More." as oppose to two different elements?

0. Baby firefoxes are wonderful and cute. 1. (Alessio is investiating the 1px thing. They look the same on OSX. Will be fixed, good catch!) 2. Two questions in one there. a. I would defer to Matej or you on whether different color and styling is enough to distinguish sentences. I claim periods for neither, but adding periods is not a big deal. (It should be 'Learn more' [lowercase m) probably though, in any case. I hate seeing periods inside links. No version of this has ever had a period for the command. b. Two different elements. Once is a message, one is an <A> link? I am confused about this critique or am misunderstanding it. People clicking on the message shouldn't open learn more (or should it? Is that the solution? PS, that actually sounds like an awesome solution, because then we could trash the learn more!) 3. Bonus question... MSG => STARS spacing. As shown: window: 12px, osx: 8px. Confirm that 12 is right :)

To summarize some re-brainstorm here :) - either this EXISTING TEXT or the ICON could be the link instead, if that is better.

Gives underline only during hover on text. No visible styling of icon. Pluses: - no rating task interference Minues: - introduces a new pattern: blindly hover / click on a pieces of the toolbar to know what they are. - less discoverable.

(Okay, it's a PITA to make the image and text linky. xul:description, and xul:image don't have href handling built in. )

(In reply to Matt Grimes [:Matt_G] from comment #22) > (In reply to Sevaan Franks [:sevaan] from comment #17) > > Is there a page ready for a "Learn more" to be linked to? > > Hey Sevaan. We created a public wiki page for Heartbeat to link to from the > release notes. > > Check it here: https://wiki.mozilla.org/Advocacy/heartbeat > > That is where the "Tell me more" or "What is this?" link would take the > user. Since Heartbeat is persistent across tabs they would see that it is > indeed from Mozilla and still have the opportunity to give us a score. How does localization of that page work? Should it appear instead on SUMO or MDN?

Please no underline. Matej, please look at this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=8573168 I proposed that the Learn more should be part of the sentence: "Please rate Firefox. Learn more." What are your thoughts? Gregg's thoughts: > a. I would defer to Matej or you on whether different color and styling > is enough to distinguish sentences. I claim periods for neither, but adding > periods is not a big deal. (It should be 'Learn more' [lowercase m) > probably though, in any case. I hate seeing periods inside links. No > version of this has ever had a period for the command. Matej you can ignore the rest of this. Gregg... > b. Two different elements. Once is a message, one is an <A> link? I am > confused about this critique or am misunderstanding it. People clicking on > the message shouldn't open learn more (or should it? Is that the solution? > PS, that actually sounds like an awesome solution, because then we could > trash the learn more!) I meant the Learn more is hyperlinked in the sentence. > > 3. Bonus question... MSG => STARS spacing. As shown: window: 12px, osx: > 8px. Confirm that 12 is right :) Mmaslaney?

To clarify, please no hover effects of any kind, except on the hyperlink as specified in the colours I showed you.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #38) > (In reply to Matt Grimes [:Matt_G] from comment #22) > > (In reply to Sevaan Franks [:sevaan] from comment #17) > > > Is there a page ready for a "Learn more" to be linked to? > > > > Hey Sevaan. We created a public wiki page for Heartbeat to link to from the > > release notes. > > > > Check it here: https://wiki.mozilla.org/Advocacy/heartbeat > > > > That is where the "Tell me more" or "What is this?" link would take the > > user. Since Heartbeat is persistent across tabs they would see that it is > > indeed from Mozilla and still have the opportunity to give us a score. > > How does localization of that page work? Should it appear instead on SUMO or > MDN? At this point Heartbeat is EN only. We'll tackle the localization hurdle when we get there, but SUMO or MDN do sound like good choices.

Matej, Mmaslaney: Please see comment 39.

(In reply to Sevaan Franks [:sevaan] from comment #39) > Please no underline. > > Matej, please look at this attachment: > https://bugzilla.mozilla.org/attachment.cgi?id=8573168 > > I proposed that the Learn more should be part of the sentence: "Please rate > Firefox. Learn more." What are your thoughts? Gregg's thoughts: > > > a. I would defer to Matej or you on whether different color and styling > > is enough to distinguish sentences. I claim periods for neither, but adding > > periods is not a big deal. (It should be 'Learn more' [lowercase m) > > probably though, in any case. I hate seeing periods inside links. No > > version of this has ever had a period for the command. I am generally in agreement on the above, though we do sometimes use "Learn more." (with the period) as a link, especially if it's part of a paragraph or follows a longer sentence. In this specific example, I think setting it off the way it is and making it blue is good enough, though I agree that it should be a lowercase m. The other option would be to put it on the other side of the stars so they're more tied to the rating, rather than the "Learn more" line. Let's also use a lowercase r in "Please rate Firefox" Thanks.

Thanks Matej. Updated mockup: http://cl.ly/image/0n1W0H0y0Z0b Note that at the end it says "What is this?" instead of Learn more. I checked with Matej over IRC that this was okay.

(In reply to Sevaan Franks [:sevaan] from comment #39) > Please no underline. > > Matej, please look at this attachment: > https://bugzilla.mozilla.org/attachment.cgi?id=8573168 > > I proposed that the Learn more should be part of the sentence: "Please rate > Firefox. Learn more." What are your thoughts? Gregg's thoughts: > > > a. I would defer to Matej or you on whether different color and styling > > is enough to distinguish sentences. I claim periods for neither, but adding > > periods is not a big deal. (It should be 'Learn more' [lowercase m) > > probably though, in any case. I hate seeing periods inside links. No > > version of this has ever had a period for the command. > > Matej you can ignore the rest of this. Gregg... > > > b. Two different elements. Once is a message, one is an <A> link? I am > > confused about this critique or am misunderstanding it. People clicking on > > the message shouldn't open learn more (or should it? Is that the solution? > > PS, that actually sounds like an awesome solution, because then we could > > trash the learn more!) > > I meant the Learn more is hyperlinked in the sentence. > > > > > 3. Bonus question... MSG => STARS spacing. As shown: window: 12px, osx: > > 8px. Confirm that 12 is right :) > > Mmaslaney? Not sure off the top of my head. Each inherited style has it's own size and line height based on the system font.

> > > 3. Bonus question... MSG => STARS spacing. As shown: window: 12px, osx: > > > 8px. Confirm that 12 is right :) > > > > Mmaslaney? > > Not sure off the top of my head. Each inherited style has it's own size and > line height based on the system font. Jaws, would you be able to chime in here? It feels like there are a lot of specified measurements in this that shouldn't be hard-coded (I could be wrong though)

(Sevaan, others! I know this was a long, long road, to get this better, and I like where it's heading. Thanks for taking a few more stabs at this!)

Alessio: 0. (This should not be added directly as a patch!) 1. added a few styles (sizing, 'de-underlining' text-link 2. moved where the label happens to after the sizer UI should be done now :)

This patch implements the requested changes (embedding Gregg's patch) and adds test coverage for the learn more link. Also, I've centered the stars vertically, as they were positioned oddly. I'm requesting a review now to speed up things, since the only issue left is the amount of space after copy.

That should be it :)

Comment on attachment 8574269 [details] Heartbeat-Windows-v4.PNG An earlier comment in this bug requested that "rate" be lower-cased (comment #43). (In reply to Sevaan Franks [:sevaan] from comment #46) > Jaws, would you be able to chime in here? It feels like there are a lot of > specified measurements in this that shouldn't be hard-coded (I could be > wrong though) Yes, different platforms/environments have different default font-sizes. It is better to use a relative unit of measurement here, such as `em`, `ch`, or `rem`. `ch` is usually the most reliable for widths of characters, as 1ch is defined as the width of the `0` character in the specified font and font-size.

Comment on attachment 8574269 [details] Heartbeat-Windows-v4.PNG Looks good. Only comment is that the "What is this?" text should have the same spacing between it and the close X as the "Please rate Firefox" text has between it and the Heartbeat logo.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #52) > Comment on attachment 8574269 [details] > Heartbeat-Windows-v4.PNG > > An earlier comment in this bug requested that "rate" be lower-cased (comment > #43). > > (In reply to Sevaan Franks [:sevaan] from comment #46) > > Jaws, would you be able to chime in here? It feels like there are a lot of > > specified measurements in this that shouldn't be hard-coded (I could be > > wrong though) > > Yes, different platforms/environments have different default font-sizes. It > is better to use a relative unit of measurement here, such as `em`, `ch`, or > `rem`. `ch` is usually the most reliable for widths of characters, as 1ch is > defined as the width of the `0` character in the specified font and > font-size. mmaslaney, any changes/recommendations based on using ch?

Comment on attachment 8574266 [details] [diff] [review] bug1137481.patch - v5 Review of attachment 8574266 [details] [diff] [review]: ----------------------------------------------------------------- There is way too much overriding of CSS for the notification bar and I let it go for the initial landing since it was going to be a temporary short-term thing but now it's turning into a bigger ordeal and more time is being spent on it. Chameleon should be implemented at the toolkit level in a separate bug so we can stop doing one-off overrides for notification bars. Until that's implemented we should live with the style we have for consistency and maintainability. ::: browser/themes/shared/UITour.inc.css @@ +161,4 @@ > } > > +/* In themes/osx/global/notification.css the close icon is inverted because notifications > + on OSX are usually dark. Heartbeat is light, so override that behaviour. */ We need to be overriding the default styles less, not more so how about we don't override the background colors at all? One-off changes are maintenance nightmares. @@ +211,3 @@ > text-shadow: none; > + -moz-margin-start: 0px; > + -moz-margin-end: 12px !important; Why do you need !important? Add a comment or remove it. @@ +237,5 @@ > +/* Learn More link styles */ > +.heartbeat > .text-link { > + color: #0095DD; > + -moz-margin-start: 0px; > + -moz-margin-end: 8px !important; Why do you need !important? Add a comment or remove it. This text-link logic should be in toolkit as part of Chameleon. @@ +247,5 @@ > +} > + > +.heartbeat > .text-link:hover:active { > + color: #006B9D; > + text-decoration: none; I don't think you need to duplicate `text-decoration: none` @@ +254,4 @@ > /* Heartbeat UI Rating Star Classes */ > .heartbeat > #star-rating-container { > display: -moz-box; > + margin-bottom: 4px; I suspect there's a better way to do this to avoid hard-coding a margin here… Are you trying to vertically center? @@ +275,5 @@ > > .heartbeat > #star-rating-container > .star-x { > background: url("chrome://browser/skin/heartbeat-star-off.svg"); > cursor: pointer; > + -moz-margin-end: 4px !important; Why do you need !important? Add a comment or remove it.

- Chameleon is an awesome goal. I look forward to it, and once it lands, ripping all this out should be part of that bug. (Is there a bug? Let's mention this there!) The faster it lands, the sooner we can pull all this out! - I am glad that annoyance about how gross this is is pushing Chameleon forward! - I tested old notification bars. These perform *much better*. If they didn't, clearly this work would be foolish. That the new designs *do work* should inform Chameleon. - Because this design rev is *also* instrumented, it will be easy to know whether to revert it, or 'try again'. - I agree that all the OSX specific stuff is gross. Most the overrides here are more like 'un-overrides', b/c OSX notification bars were filled with overrides. Figuring it out was horrendous. - I hope the 'one-off-ness' is 'contained' here. It's in two files, to cover one very specific case. - This is the test case for new notifacation bar designs. At every stages, the effectiveness of these designs on response rate is measured. Going through these iterations is (I hope) actually influencing what will be included in chameleon. Technical Note: - (we might be over importanting here. Understanding the css cascade for these bars (on OSX in particular) is Hard (for me at least!).

(In reply to Gregg Lind (User Advocacy - Heartbeat - Test Pilot) from comment #56) > - Chameleon is an awesome goal. I look forward to it, and once it lands, > ripping all this out should be part of that bug. (Is there a bug? Let's > mention this there!) The faster it lands, the sooner we can pull all this > out! I don't know of a bug and it's not clear what the current status is so I would rather not rely on it. Dozens of other notification bars are fine with the default style. > - I am glad that annoyance about how gross this is is pushing Chameleon > forward! I don't know that it really is unless someone actually works on it. I was against the one-off for translation from the beginning but that was around a year ago probably and we still are stuck with the tech-debt. > - I tested old notification bars. These perform *much better*. If they > didn't, clearly this work would be foolish. That the new designs *do work* > should inform Chameleon. I didn't know you had tested old notification bars. This is all the more reason to implement Chameleon for all notification bars. > - Because this design rev is *also* instrumented, it will be easy to know > whether to revert it, or 'try again'. I thought you just said above you already had data that these are better. We also already used Chamelon for translation notification bars. Why do we need more data? > - I agree that all the OSX specific stuff is gross. Most the overrides here > are more like 'un-overrides', b/c OSX notification bars were filled with > overrides. Figuring it out was horrendous. That's because we're trying to fit in with the platform and that's why we have separate theme directories for each platform. > - I hope the 'one-off-ness' is 'contained' here. It's in two files, to > cover one very specific case. It's not contained when we make fixes to the notification bars e.g. bug 1047977 (which has to be special-cased for the 2 Chameleon notification bars). > - This is the test case for new notifacation bar designs. At every stages, > the effectiveness of these designs on response rate is measured. Going > through these iterations is (I hope) actually influencing what will be > included in chameleon. That seems like scope creep on Heartbeat and an excuse to plow though with adding technical debt. We already had the translation notifications to test out Chameleon if we wanted to. > Technical Note: > - (we might be over importanting here. Understanding the css cascade for > these bars (on OSX in particular) is Hard (for me at least!). Unnecessary !important adds technical debt too and makes it harder for add-ons to theme so that's what I want to make sure they're necessary.

(MattN, point us to existing Notification bars that are in Chameleon style. This is news to me, and might have changed things had we known earlier.)

Comment on attachment 8574266 [details] [diff] [review] bug1137481.patch - v5 Review of attachment 8574266 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/shared/UITour.inc.css @@ +163,5 @@ > +/* In themes/osx/global/notification.css the close icon is inverted because notifications > + on OSX are usually dark. Heartbeat is light, so override that behaviour. */ > + > +%ifdef XP_MACOSX > +notification.heartbeat[type="info"]:not([value="translation"]) .close-icon:not(:hover) { Remove :not([value="translation"]) on these since you won't ever be both a heartbeat and a translation notification.

Thanks for reviewing this, MattN. > > @@ +254,4 @@ > > /* Heartbeat UI Rating Star Classes */ > > .heartbeat > #star-rating-container { > > display: -moz-box; > > + margin-bottom: 4px; > > I suspect there's a better way to do this to avoid hard-coding a margin > here… Are you trying to vertically center? Yes, I'm trying to align the stars vertically, with the content of the notification description (the text).

Comment on attachment 8576001 [details] [diff] [review] bug1137481.patch - v6 Unfortunately, I could not find a better way to center the stars container vertically, as the hbox isn't taking all the space: for this reason I could not make -moz-box-align and -moz-box-pack. I left the margin-bottom:4px, as done in [1]. https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/global.css#235

Try push for the attached patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe03667ead88

Comment on attachment 8576001 [details] [diff] [review] bug1137481.patch - v6 Approval Request Comment [Feature/regressing bug #]: 1111016 [User impact if declined]: Heartbeat UI will have the older look. As a consequence, it will probably be disabled. [Describe test coverage new/current, TreeHerder]: Adds new mochitests. There were no problems during its time on m-c. [Risks and why]: Low risk, limited to the Heartbeat UI (the changed styles are only used by heartbeat). [String/UUID change made/needed]:

Comment on attachment 8576001 [details] [diff] [review] bug1137481.patch - v6 Our final Beta is building on Thursday. It's too late to take non critical changes. If Heartbeat can't be enabled because of this, we'll need to push the enablement of this feature to Firefox 38. Beta-

(This can land whenever, and is robust to 'partial deploy', Thanks Lawrence, no worries!)