To meet the acceptance criteria as stated in bug 1179164, we should: - Implement the new style sharing buttons. Do not add the buttons that we don't have an implementation for (yet). - Implement a new context menu (information popover) that informs the user that the link was copied. This link will fade out and hide after 2 seconds. - Make sure that the conversation toolbar is visible on top of the overlay. For the visual design spec, please check out the ones attached to bug 1179164.

Sevaan, what should we do with 'Add some context' link that is visible when you open a conversation without context attached? (It's the one with the pencil icon in front of it.)

(In reply to Mike de Boer [:mikedeboer] from comment #1) > Sevaan, what should we do with 'Add some context' link that is visible when > you open a conversation without context attached? (It's the one with the > pencil icon in front of it.) That link has been moved to the gear menu: http://i.sevaan.com/image/393d2T103U2F

(In reply to Sevaan Franks [:sevaan] from comment #2) > That link has been moved to the gear menu: > http://i.sevaan.com/image/393d2T103U2F Fair enough, I thought perhaps that might not be prominent enough...

Visibility is not really a concern for the moment. In the near future context won't work the way it does not, with just being a URL attached to the room. As we move forward in web-sharing, adding context is just bringing a page into your web sharing window, and we are already exploring mechanisms to do that.

Just for clarification, the "Submit Feedback" option in the gear menu on http://i.sevaan.com/image/393d2T103U2F is not required.

Bug 1178304 will add the facebook button. The buttons shown on the invite view: (share on Facebook) (share with contacts) (copy link) (?unlabeled email link) What should happen when clicking on the contacts button "share with contacts" that seems to show a arrowpanel with contacts and their email addresses. Does this include all contacts? These round buttons have a static blue #00a9dc and hover light blue #5cccee and an active-for-10-seconds green #56b397 each with a white glyph/icon.

ni? for the images for the buttons: https://www.dropbox.com/sh/46pyq5wwgnif6g8/AACEqjLwDw1be0oMYw-okHA9a?dl=0&preview=Share_Screen.png

Michael, Would you be able to provide these assets? http://i.sevaan.com/image/2w2i0C3S2x42 The Facebook icon can be found here: https://www.facebookbrand.com/ But the rest are just fontawesome glyphs so I suspect they can't be used.

Separate from icons, there's the new "share with contacts" button and arrowpanel. Should that be added in this bug as it's new functionality that doesn't seem to be defined? In particular what should appear in the arrowpanel and what should happen when clicking on the button?

No, no need to implement that. It appears Contacts will be going away in forthcoming releases.

How opaque should the overlay be? It used to be 60%. I changed it to 90% for this screenshot. What should the text be for hovering over email and after clicking? I assumed "email link" and "emailed!" What should happen if the user clicks on one button causing the text to appear for a few seconds but then hovers over the other button causing two sets of button text to appear? Potentially they could overlap depending on how close we place the buttons. (But in the screenshot, both happen to be spaced apart enough.) On a related note how should the buttons be spaced? Right now they're just equally spaced (flex). (I've put some placeholder images for the button for now.)

We can land a string-only patch by taking the .properties file and only including the additions (don't remove to-be-unused strings as they're still used).

Comment on attachment 8663085 [details] [diff] [review] added strings patch Review of attachment 8663085 [details] [diff] [review]: ----------------------------------------------------------------- rs=dmose

https://hg.mozilla.org/integration/fx-team/rev/9d8166cf45a2bc6b1c5d1dab4dc6dc37214dad0d Bug 1184924 - Implement the refreshed design for the invitation overlay strings [rs=dmose]

Where can I see specs/visual to understand how strings are used? I'm quite surprised by the lowercase. As a side note, I don't really understand why we host specs on external services instead of using Bugzilla's attachments, at least for low-res versions. The direct result of this decision is that the link provided in comment 7 is broken, and that happens a lot in Loop bugs.

(In reply to Ed Lee :Mardak from comment #11) > Created attachment 8663052 [details] > wip screenshot > > How opaque should the overlay be? It used to be 60%. I changed it to 90% for > this screenshot.t Let's do it at 85%, thanks! > What should the text be for hovering over email and after clicking? I > assumed "email link" and "emailed!" I don't think it needs to change. There is a change of state behind the scenes with copying to a clipboard. But when clicking the email button it's obvious what the change is, and I can do it multiple times. I don't think we need the "Shared!" either for sharing with Facebook now that I'm looking at the spec. > What should happen if the user clicks on one button causing the text to > appear for a few seconds but then hovers over the other button causing two > sets of button text to appear? Potentially they could overlap depending on > how close we place the buttons. (But in the screenshot, both happen to be > spaced apart enough.) We should instantly revert back, so the action text goes back to it's original state instantly. > On a related note how should the buttons be spaced? Right now they're just > equally spaced (flex). All buttons should be centered with 8px gap between them. (In reply to Francesco Lodolo [:flod] from comment #18) > Where can I see specs/visual to understand how strings are used? I'm quite > surprised by the lowercase. I agree here. Can we have the first letter uppercase, please? I'll attach a more up-to-date spec to this bug to address dead linkage with Dropbox.

Here's the design I've been working with from the now-dead link. If we do want to make the first letter uppercase, we'll need to get it in really soon/today.

(In reply to Ed Lee :Mardak from comment #20) > If we do want to make the first letter uppercase, we'll need to get it in > really soon/today. I think we're already past that, merges are happening as we speak, so this will go into fx44.

Ignoring the icon for now.. clicking on the button and waiting 2 seconds for the triggered state to go away.. then clicking again then hovering over the other button.

Assets from :mmaslaney attached.

(In reply to Sevaan Franks [:sevaan] from comment #25) > Created attachment 8664198 [details] > Invitation Assets (SVG) > Assets from :mmaslaney attached. These should be inverted the other way. There should be white for the icon and transparent for the background (and no need for the circle). E.g., just a white mail icon with no/transparent background.

New assets attached

(In reply to Michael Maslaney [:mmaslaney] (mmaslaney@mozilla.com) from comment #27) > Created attachment 8664377 [details] > Hello_Assets_092015-v2.zip > New assets attached Thanks! Looks good!

Comment on attachment 8664748 [details] [diff] [review] v1 Review of attachment 8664748 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. r=Standard8 with the comments addressed. Note that Sevaan asked for the strings to start with capitals in comment 19. Can you add a patch to change that please? ::: browser/components/loop/content/js/roomViews.jsx @@ -286,2 @@ > </p> > - <a className={cx({hide: !canAddContext, "room-invitation-addcontext": true})} There's some room-invitation-addcontext classes that need removing: http://mxr.mozilla.org/mozilla-central/search?string=room-invitation-addcontext @@ +297,5 @@ > + })} > + onClick={this.handleCopyButtonClick}> > + <img src="loop/shared/img/svg/glyph-link-16x16.svg" /> > + <p>{mozL10n.get("invite_copy_" + > + (this.state.copiedUrl ? "triggered" : "button"))}</p> nit: I think I'd prefer two-spaces more indent here. ::: browser/components/loop/test/desktop-local/roomViews_test.js @@ +273,2 @@ > > + it("should keep the text soon after the url has been copied", function() { nit: possibly s/soon/for a while/

(Please make the l10n changes a separate patch, as we'll want the non-L10n stuff for uplift).

Comment on attachment 8667685 [details] [diff] [review] for checkin I updated this patch with review comments and removed the .properties string deletion to a separate patch.

Comment on attachment 8667686 [details] [diff] [review] strings update Hrmm.. With the new uppercase string, it's the same as the one being removed, so the code should probably just use the old string: invite_copy_triggered2=Copied! copied_url_button=Copied! And should we get rid of the other "triggered" strings as it seems like they won't be used? And "Share with contacts" too.. These are pretty close as well: +invite_copy_button2=Copy link copy_url_button2=Copy Link

Alternatively use the panel strings... which probably is not be good for l10n as the panel strings are used in a menu while the new functionality is labeled buttons. Somewhat interesting to note: panel's menu strings are.. Copy Link Email Link Delete conversation So there's some casing inconsistency here. And reusing the strings isn't quite right as I believe we want "Copy link" and "Email link"

Firstly: Sevaan, should we use "Copy Link" (similar to what's in the menus in the panel for conversations), or "Copy link"? I think this is a slightly different case as it isn't in a menu, so "Copy link" might make more sense, but I'll let Sevaan decide. Secondly: Our strings here are a mess here. I would be happy for you to spin some or all of fixing this out to a separate bug (though we should make it high prio). I think what we should do is: - Make the strings in the panel for the menus be consistent and next to each other in the file, e.g. copy_link_menuitem email_link_menuitem delete_conversation_menuitem - Make the conversation window strings consistent (append '2' if necessary), e.g. invite_copy_link_button invite_copied_link_button ... I think button is about the right sort of term here, though we could use label/tooltip instead maybe (though its not quite any of those). That will give L10n the context as to where these strings live, as there may be a few instances where they have to change the string.

Comment on attachment 8667686 [details] [diff] [review] strings update Review of attachment 8667686 [details] [diff] [review]: ----------------------------------------------------------------- This is probably reasonably close to what we want, but I'll let you take a look at the comments & Sevaan's response when we get it, before I review this.

(In reply to Mark Banner (:standard8) from comment #37) > Firstly: Sevaan, should we use "Copy Link" (similar to what's in the menus > in the panel for conversations), or "Copy link"? Copy Link, please.

https://hg.mozilla.org/integration/fx-team/rev/63fe244f62b8ab4b7afe560b41a8bcf1a62afcaa Bug 1184924 - Implement the refreshed design for the invitation overlay [r=Standard8]

Comment on attachment 8667686 [details] [diff] [review] strings update This patch was handled in bug 1210331.

Comment on attachment 8667730 [details] [diff] [review] using panel strings This patch was handled in bug 1210331.

Comment on attachment 8667685 [details] [diff] [review] for checkin This patch as attached is upliftable to aurora if bug 1209029 is *not* uplifted (there's merge conflicts with jar.mn and various glyph svgs being removed) and if bug 1211438 *is* uplifted (there's a potential merge conflict with conversation.css). Standard8, do you want me to attach a patch that can be uplifted to mozilla-aurora right now? (I.e., without bug 1211438 uplifting first)

Approval Request Comment [Feature/regressing bug #]: Hello Visual refresh [User impact if declined]: Old text-button interface instead of new icons with matching colors. [Describe test coverage new/current, TreeHerder]: Various unit tests landed on m-c and baked for about a week [Risks and why]: Lowish - mostly css changes and reusing existing existing styles [String/UUID change made/needed]: None - strings landed when m-c was 43 I'll land this after bug 1211438 is uplifted.

Comment on attachment 8670936 [details] [diff] [review] for aurora (Mardak will land) Approved for aurora uplift, has tests, part of hello feature aimed at 43.

Tracking since this is part of new feature work.

http://hg.mozilla.org/releases/mozilla-aurora/rev/6424691f9e9a Bug 1184924 - Implement the refreshed design for the invitation overlay [r=Standard8, a=lizzard]

Verified that the new overlay works as expected using Firefox Developer Edition 43.0a2 and latest Nightly 44.0a1 across platforms (Windows 7 64-bit, Mac OS X 10.11 and Ubuntu 14.04 32-bit).