Here's an outline of the approach we'll take, including the one edge case we think we'll have to live with (at least for 39). Flows: #1 User has never clicked "Share Link" in Hello - User clicks "Share Link" - Share panel opens (anchored to hello button), showing a list of pre-selected providers - User clicks a provider, provider automatically loads and user can complete the share, share button is not moved into toolbar - User completes sharing the link #2 User has previously used Hello and shared links via "Share Link" button - User clicks "Share Link" - A submenu with (previously activated) providers appears - User clicks on a provider in the submenu - Panel appears anchored to Hello button (even if the share button was previously moved into the toolbar) - User completes sharing the link #3 User has never clicked share link in hello (edge case) - User clicks "Share Link" - Share panel opens attached to Hello button, showing a list of pre-selected providers - User clicks "View more" - Panel closes, activation directory is loaded in panel - User activates a provider, share button *IS* moved into toolbar - User has to start over again by clicking "Share Link" in Hello Note: completing the share from hello in this case is a more complex change than we'd probably want to uplift into beta Changes to share: - Share panel becomes "targetable" so it can be anchored to the Hello button if initiated from Hello - The "instant activate" panel that shows some default providers gets pref'd on for release. This is the panel you see in the google doc Romain posted. - If the panel is open during activation, the share button is not moved into the toolbar. Changes to hello: - The "add share panel to my toolbar" panel in hello is removed - If there are no providers installed, the "Share Link" button does not show a submenu, it just opens the share panel, this removes an extra click

Comment on attachment 8614029 [details] [diff] [review] Patch v1: social panel anchoring changes and more I just realized why I had the activations.cdn.mozilla.net domain in the prefs, we'll need to leave those alone for now (sorry for the confusion). When the cdn changes happen next Q we'll have to update these. This looks great, I'll try to get time to apply the patch and give it a spin. IMO lets run this through try and land it.

found two issues. one is illustrated in the attached image. 1. If the share button is in the toolbar, it gets depressed when hello opens the panel. 2. This is unrelated to this patch, could be another bug, I'm being lazy. From a fresh profile, doing my first hello share, there is a panel opened to the left of the hello window that says something about sharing links. I click on "Share Link" in the hello window, the share panel opens (attached to hello toolbar button) but that little side panel is not closed (it should).

(In reply to Shane Caraveo (:mixedpuppy) from comment #3) > 2. This is unrelated to this patch, could be another bug, I'm being lazy. > From a fresh profile, doing my first hello share, there is a panel opened to > the left of the hello window that says something about sharing links. I > click on "Share Link" in the hello window, the share panel opens (attached > to hello toolbar button) but that little side panel is not closed (it > should). That's a bug with FTU, is my guess.

Shane, could you sign-off on the Social API code changes? I'll be pushing to try shortly. Mark, I do feel it's necessary for you to look at a bit. Don't worry, it's mostly code removal :-P

Comment on attachment 8614590 [details] [diff] [review] Patch v2: social panel anchoring changes and more looks great. If we can use panel.anchor instead of keeping a "currentAnchor" reference lets do that.

(In reply to Shane Caraveo (:mixedpuppy) from comment #7) > If we can use panel.anchor instead of keeping a > "currentAnchor" reference lets do that. ignore that, I wasn't through my first cup of coffee

Comment on attachment 8614590 [details] [diff] [review] Patch v2: social panel anchoring changes and more Review of attachment 8614590 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/test/desktop-local/roomViews_test.js @@ +236,5 @@ > React.addons.TestUtils.Simulate.click(shareBtn); > > expect(view.state.showMenu).to.eql(true); > expect(view.refs.menu.props.show).to.eql(true); > + }) I don't think you want this change - eslint says missing semi-colon.

Comment on attachment 8614590 [details] [diff] [review] Patch v2: social panel anchoring changes and more Approval Request Comment [Feature/regressing bug #]: hello link sharing [User impact if declined]: overly complex ux [Describe test coverage new/current, TreeHerder]: current mochitests cover this [Risks and why]: low [String/UUID change made/needed]: none iiuc, the reason for the rush on this work was to fix the UX that is in 39, and this needs to be uplifted. Making the request now to get the ball moving, but am not the owner on the feature. need-info RT to make sure uplifting to 39 is correct.

This looks correct. We want to fix the UX since the current implementation gives an incentive to add the shareplane button to the toolbar whereas there has been a decision made recently to stop promoting the shareplane button since a different solution will soon be implemented in the browser. The patch proposed for uplift will help avoid getting more shareplane button users to help make the transition simpler when the alternative sharing solution gets implemented.

Comment on attachment 8614590 [details] [diff] [review] Patch v2: social panel anchoring changes and more I will take it for aurora as we would have time to fix potential regressions. I am not sure we want that in 39. This is late in the cycle (we don't have many beta left as it is a shorter cycle). What about shipping 39 in the current state and improve it in 40?

(In reply to Sylvestre Ledru [:sylvestre] from comment #15) > I will take it for aurora as we would have time to fix potential regressions. > I am not sure we want that in 39. This is late in the cycle (we don't have > many beta left as it is a shorter cycle). > What about shipping 39 in the current state and improve it in 40? I'm afraid that's not acceptable for the UX team. The choice they presented us was basically: make these adjustments or back out the feature entirely. Since we figured that doing this change would be cheaper than a backout, we prepared this patch.

Got the nod from Shane over IRC to drop the loop.properties change to keep l10n happy. https://hg.mozilla.org/releases/mozilla-aurora/rev/afe619271392

Neither option sounds good for this stage of beta 39. I don't think that risking slipping the release, or even having to do a point release, is worth the future awkwardness of helping to make the transition simpler to replace this button with something else. Backing out the entire feature also sounds liable to break something unexpected and to be a distraction from fixing the stability issues we need to be focused on in beta. I don't think we have time to fix, or even find, problems in this feature change with a week and a half left till we go to build with 39 RC. Since it's in 40 at this point, we can let it ride with 40 and it will be released in mid-August. Who should I be talking to in UX? Romain? Is this level of fluctuation in the feature likely to keep happening? If so we need to plan around it in some more sustainable way than doing this at the end of beta.

I'm OK with this. Just talked to Sevaan from UX and this is fine with them too (Madhava is also OK with this). So let's keep this unchanged as you suggest in 39. This is happening quite late because we realized late that what is being implemented in Hello contradicts some of the long term UX plans - this is unfortunate, we'll make sure we have tighter feedback loops between overall Firefox UX and Hello teams to avoid this in the future!

OK. Thank you Romain. What you say about feedback loops makes great sense!

Verified that the flow that was implemented in this bug is working as expected using latest Aurora and Nightly across platforms (Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 14.04 32-bit), no new issues were found.