Closed Bug 1047144 Opened 5 years ago Closed 5 years ago

Implement the gear menu in the Loop panel footer

Categories

(Hello (Loop) :: Client, defect)

defect
Not set

Tracking

(firefox35 verified)

VERIFIED FIXED
mozilla35
Tracking Status
firefox35 --- verified

People

(Reporter: MattN, Unassigned)

References

()

Details

(Whiteboard: [p=1, strings][loop-uplift])

Attachments

(1 file, 4 obsolete files)

The menu allows for function such as Sign In and Sign Out.
Flags: firefox-backlog+
Points: --- → 5
Whiteboard: [qa+] → [qa+] [strings]
Hey Darrin,

Where should we get the assets for the gear icon and the menu items? In your spec they are using a font it seems but I'm not sure if that's what we're supposed to use too?
Flags: needinfo?(dhenein)
All of the assets are attached here: https://bugzilla.mozilla.org/show_bug.cgi?id=1016087


No FontAwesome in the UI ;)
Flags: needinfo?(dhenein)
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Iteration: --- → 34.3
Flags: qe-verify+
Whiteboard: [qa+] [strings] → [strings]
QA Contact: anthony.s.hughes
Whiteboard: [strings] → [p=1, strings]
Rebalancing bugs now that some folks are back from PTO.  I talked with Paolo, Niko, and MattN, and it makes sense for Niko to take this bug.
Assignee: paolo.mozmail → nperriault
Iteration: 34.3 → ---
Points: 5 → ---
Flags: firefox-backlog+
Attached patch Added gear menu to Loop panel. (obsolete) — Splinter Review
This patch adds the "gear menu" to the Loop panel; this is a WiP as it misses:

- the sign in/sign out icons, I've commented about them here https://bugzilla.mozilla.org/show_bug.cgi?id=1016087#c7
- the sign in, sign out and authentication status check methods are stubs for now, so any auth attept with fail with an "undefined is not a function" error

But I think this should help :MattN testing the bugs he's working on. We'll merge everything at once when it's ready.
Attachment #8479915 - Flags: feedback?(standard8)
Attachment #8479915 - Flags: feedback?(MattN+bmo)
Comment on attachment 8479915 [details] [diff] [review]
Added gear menu to Loop panel.

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

::: browser/components/loop/content/js/panel.jsx
@@ +161,5 @@
> +    handleClickAuthButton: function() {
> +      if (navigator.mozLoop.loggedInToFxA) { // XXX to be implemented
> +        navigator.mozLoop.logOutFromFxA();   // XXX to be implemented
> +      } else {
> +        navigator.mozLoop.logInToFxA();      // XXX to be implemented

This is being added in bug 1059021.

::: browser/locales/en-US/chrome/browser/loop/loop.properties
@@ +67,5 @@
>  copied_url_button=Copied!
> +
> +settings_menu_item_signout=Sign out
> +settings_menu_item_signin=Sign in
> +settings_menu_button_tooltip=Settings

Did you intentionally omit the "Account" option from https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#precall-signedin
I'm not sure what it's supposed to do or whether it will make 34 but just wondering.
Comment on attachment 8479915 [details] [diff] [review]
Added gear menu to Loop panel.

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

The general concepts look fine to me. I might review in a bit more detail once I can play with it (doesn't apply currently).
Attachment #8479915 - Flags: feedback?(standard8) → feedback+
Attachment #8479915 - Flags: feedback?(MattN+bmo)
Will you be able to pre-land this before Tuesday so the strings get in?  If you need help getting these strings in, let me and Mark (Standard8) know.
Flags: needinfo?(MattN+bmo)
I assume the gear menu does not expose any way to access to the settings panel as the settings panel won't be implemented.
I am unclear about whether the gear menu will let the signed-in user access his account as it is part of the UX from Darrin but I don't see explicit confirmation on that thread. Can someone please confirm?
Rebased & updated patch to include signin and signup icons in the gear menu. The patch should apply cleanly on top of latest fx-team.

The patch is still incomplete (WiP) as we miss a way to be notified that the auth status of the user has changed, so we can trigger a new rendering of the whole panel component.

Last, I fixed a previous regression with the UI component Showcase which was displaying the panel footer using an erroneous font size (so I removed obsolete css rules for footers in the webapp.css file).
Attachment #8479915 - Attachment is obsolete: true
Attachment #8481331 - Flags: feedback?(standard8)
Attachment #8481331 - Flags: feedback?(MattN+bmo)
Comment on attachment 8481331 [details] [diff] [review]
Added gear menu to Loop panel. (WiP)

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

::: browser/locales/en-US/chrome/browser/loop/loop.properties
@@ +73,5 @@
>  panel_footer_signin_or_signup_link=Sign In or Sign Up
> +
> +settings_menu_item_signout=Sign out
> +settings_menu_item_signin=Sign in
> +settings_menu_button_tooltip=Settings

I think we should at least add the account string so we have it for 34 if needed. Can you make sure there is a follow-up filed for adding it?
Updated patch with added strings for everything gear menu according to :darrin's spec for MVP:

- "Settings"
- "Account"
- "Sign In"
- "Sign Out"
Attachment #8481331 - Attachment is obsolete: true
Attachment #8481331 - Flags: feedback?(standard8)
Attachment #8481331 - Flags: feedback?(MattN+bmo)
Attachment #8481524 - Flags: feedback?(standard8)
For the "Account" entry, I didn't find anything. NI :RT here.
Flags: needinfo?(rtestard)
Niko, the idea was to link to the FxA accounts page (open a new tab and get to that page).
I am unclear about state of readiness from the FxA team on this so as discussed on IRC lets land the string and hide it from the gears menu.
I NI Chris K for an update on readiness on the FxA account management page.
Flags: needinfo?(rtestard) → needinfo?(ckarlof)
Until FxA is ready and we've done integration testing, we'll be hiding the FxA implementation. 

In terms of the strings, MattN is handling the landing of all the FxA bugs that have strings, including this bug and a few others, to make sure we don't miss anything or enable something before it's ready.  So please keep him in the loop for anything related to landing.

Matt -- Do you have all the info you need for the strings in this bug?

Standard8 is helping with all string uplifts, so I'm needinfo'ing him so he knows about this conversation and in case he has questions or info for us about this.
Flags: needinfo?(standard8)
Attached patch Added gear menu to Loop panel. (obsolete) — Splinter Review
Rebased patch on top of latest fx-team; the gear menu, while fully implemented from a UI perspective, is fully hidden until bug 979845 (and possibly others) lands.

Also, we'll want to ensure we have events to listen to for when authentication state changes, so we can trigger rerendering of the panel footer — including the gear menu with updated links accordingly.
Attachment #8481524 - Attachment is obsolete: true
Attachment #8481524 - Flags: feedback?(standard8)
Attachment #8482280 - Flags: review?(standard8)
Forgot to add missing changes & assets, sorry for bugnoise.
Attachment #8482280 - Attachment is obsolete: true
Attachment #8482280 - Flags: review?(standard8)
Attachment #8482282 - Flags: review?(standard8)
Attachment #8482282 - Flags: review?(standard8) → review+
I've landed this, is Matt wants to add more he's welcome to (hence leaving needinfo), otherwise this should be all we need in this menu for now:

https://hg.mozilla.org/integration/fx-team/rev/05cbe9a31ad3
Flags: needinfo?(standard8)
Target Milestone: --- → mozilla34
I don't think more needs to be done in this bug. We can handle unhiding the UI and making it work in a separate bugs.
Flags: needinfo?(MattN+bmo)
https://hg.mozilla.org/mozilla-central/rev/05cbe9a31ad3
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
(In reply to Matthew N. [:MattN] from comment #18)
> I don't think more needs to be done in this bug. We can handle unhiding the
> UI and making it work in a separate bugs.

Is there a way for me to unhide the UI with prefs? At least then I can verify this as part of the iteration.
No, sorry. I guess verification would have to wait until the login UI is visible.
(In reply to Matthew N. [:MattN] from comment #21)
> No, sorry. I guess verification would have to wait until the login UI is
> visible.

Thanks for clarifying. I'm going to untrack this for verification since it's not currently testable. That said, I'll make sure this gets some smoketest coverage before Firefox 34 is finally signed off.
Flags: qe-verify+ → qe-verify-
FxA settings are at: https://accounts.firefox.com/settings

We link to this page from the Sync pref panel.
Flags: needinfo?(ckarlof)
FYI, the above URL is available in the pref, identity.fxaccounts.settings.uri
Whiteboard: [p=1, strings] → [p=1, strings][loop-uplift]
This can be verified starting with tomorrow's Nightly now that bug 1065144 landed.
Flags: qe-verify- → qe-verify+
The gear menu is properly displayed in the Loop panel footer.
Verified fixed 35.0a1 (2014-09-29) Win 7
Status: RESOLVED → VERIFIED
I think the target milestone is incorrect since this landed in Firefox 35.0a1. Is this needed for 34?
Target Milestone: mozilla34 → mozilla35
You need to log in before you can comment on or make changes to this bug.