Closed
Bug 1059756
Opened 10 years ago
Closed 9 years ago
Desktop client UI needs to display a link to the SUMO page for help
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox35 fixed, firefox36 fixed)
backlog | Fx35+ |
People
(Reporter: RT, Assigned: rgauthier)
References
()
Details
User Story
As a desktop client user I want to be able to access the support pages so that I can get help on issues I may have.
Attachments
(3 files, 5 obsolete files)
18.78 KB,
image/png
|
Details | |
815 bytes,
image/svg+xml
|
Details | |
10.90 KB,
patch
|
standard8
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
No description provided.
Updated•10 years ago
|
Priority: -- → P1
Comment 1•10 years ago
|
||
sorry if i missed it - but i didn't see a link for help in the Room design. Can that be added? Not sure if something as simple as a ? would work - but if yes, "YAY" as that doesn't need translation (if we end up uplifting part of the Rooms UX after 35 Aurora deadline).
Flags: needinfo?(dhenein)
Comment 2•10 years ago
|
||
One suggestion (and my recommendation) would be to put it in the menu, alongside settings and log in/out. Does this satisfy your user story? See attachment.
Flags: needinfo?(dhenein)
Comment 3•10 years ago
|
||
Updated•10 years ago
|
backlog: --- → Fx36+
Summary: Desktop client UI needs to display a link to the SUMO page → Desktop client UI needs to display a link to the SUMO page for help
Updated•10 years ago
|
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → rgauthier
Comment 5•10 years ago
|
||
Darrin or Mike -- Do we have an svg icon for Help (as shown in the mockup)? Romain (tOkeshu) is ready to implement this today, but we need to know where the asset is. Thanks.
Flags: needinfo?(mmaslaney)
Flags: needinfo?(dhenein)
Comment 6•10 years ago
|
||
Hi Arcadio -- We need a SUMO page URL for Fx35 (which uses Rooms). The only SUMO page I know about is https://support.mozilla.org/en-US/kb/firefox-hello-make-and-receive-calls-guest-mode (which applies only to Fx34). Romain is implementing this today, so we need a URL (even if the content for now is very basic). The plan is to land this bug fix in Nightly (Fx36) and uplift it to Fx35. Thanks.
Flags: needinfo?(alainez)
Comment 7•10 years ago
|
||
Will ping Mike about this today, he has been handling all the assets for Hello.
Flags: needinfo?(dhenein)
Comment 8•10 years ago
|
||
SUMO page URL: https://support.mozilla.org/kb/group-conversations-firefox-hello-webrtc taken from standalone SUMO bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1059744#c5
Flags: needinfo?(alainez)
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8523888 -
Flags: review?(standard8)
Attachment #8523888 -
Flags: review?(nperriault)
Comment 11•10 years ago
|
||
Comment on attachment 8523888 [details] [diff] [review] Bug 1059756: Add a link to Hello's SUMO page Review of attachment 8523888 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, just a couple of minor things to address. I'm tempted to say we should make those menu items share an svg file, but lets look at that after the standalone side has the help item as well, as I think that may suggest we keep it separate. ::: browser/components/loop/content/js/panel.jsx @@ +241,5 @@ > } > }, > > + handleHelpEntry: function(event) { > + window.open("https://support.mozilla.org/kb/group-conversations-firefox-hello-webrtc"); This should be split out to a preference in firefox.js for easy changing in future - e.g. see http://mxr.mozilla.org/mozilla-central/search?string=legal.ToS_url&find=&findi=&filter=^%5B^\0%5D*%24&hitlimit=&tree=mozilla-central @@ +277,5 @@ > __("settings_menu_item_signin")} > onClick={this.handleClickAuthEntry} > displayed={navigator.mozLoop.fxAEnabled} > icon={this._isSignedIn() ? "signout" : "signin"} /> > + <SettingsDropdownEntry label={__("settings_menu_item_help")} This should currently be just "help_label" - as that's the existing string we've got for this and we need to uplift it to 35 (where we can't change strings).
Attachment #8523888 -
Flags: review?(standard8)
Attachment #8523888 -
Flags: review?(nperriault)
Attachment #8523888 -
Flags: review-
Assignee | ||
Comment 12•10 years ago
|
||
Attachment #8523888 -
Attachment is obsolete: true
Attachment #8524630 -
Flags: review?(standard8)
Comment 13•10 years ago
|
||
Comment on attachment 8524630 [details] [diff] [review] Bug 1059756: Add a link to Hello's SUMO page Review of attachment 8524630 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay in getting to this, could you update it for bitrot please? There's a couple of things changed as well. I haven't been able to test this yet due to the bitrot - does the panel close when you click the help item? If it doesn't currently, you can just add window.close() and it'll work fine for the panel (there's a special hook to prevent it actually being closed). Oh, we also should really have a test for this action - there should be similar ones you can crib off. ::: browser/components/loop/content/js/panel.jsx @@ +241,5 @@ > } > }, > > + handleHelpEntry: function(event) { > + var helloSupportUrl = navigator.mozLoop.getLoopCharPref('support_url'); getLoopCharPref should now just be getLoopPref @@ +243,5 @@ > > + handleHelpEntry: function(event) { > + var helloSupportUrl = navigator.mozLoop.getLoopCharPref('support_url'); > + window.open(helloSupportUrl); > + event.preventDefault(); nit: please put the preventDefault first
Attachment #8524630 -
Flags: review?(standard8)
Assignee | ||
Comment 14•10 years ago
|
||
Attachment #8524630 -
Attachment is obsolete: true
Attachment #8526027 -
Flags: review?(standard8)
Comment 15•10 years ago
|
||
Comment on attachment 8526027 [details] [diff] [review] Bug 1059756: Add a link to Hello's SUMO page Review of attachment 8526027 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/loop/content/js/panel.jsx @@ +328,5 @@ > displayed={navigator.mozLoop.fxAEnabled} > icon={this._isSignedIn() ? "signout" : "signin"} /> > + <SettingsDropdownEntry label={mozL10n.get("help_label")} > + onClick={this.handleHelpEntry} > + displayed={true} @displayed is not necessary IIRC ::: browser/components/loop/content/shared/css/panel.css @@ +660,5 @@ > background: transparent url(../img/svg/glyph-signout-16x16.svg) no-repeat center center; > } > > +.settings-menu .icon-help { > + background: transparent url(../img/svg/glyph-help-16x16.svg) no-repeat center center; We really should fix this at some point to only change the background-image for each icon and have the rest of the background rules shared. ::: browser/components/loop/content/shared/img/svg/glyph-help-16x16.svg @@ +1,2 @@ > +<?xml version="1.0" encoding="utf-8"?> > +<!-- Generator: Adobe Illustrator 18.1.0, SVG Export Plug-In . SVG Version: 6.00 Build 0) --> Missing license header
Attachment #8526027 -
Flags: review?(standard8) → review+
Comment 16•10 years ago
|
||
addressed nits and modified patch so it works inside chrome
Updated•10 years ago
|
Attachment #8526027 -
Attachment is obsolete: true
Updated•10 years ago
|
Assignee: rgauthier → mreavy
Status: NEW → ASSIGNED
Comment 17•10 years ago
|
||
updated tests to match code change
Updated•10 years ago
|
Attachment #8527213 -
Attachment is obsolete: true
Comment 18•10 years ago
|
||
Comment on attachment 8527226 [details] [diff] [review] [PATCH] : Add a link to Hello's SUMO page Tested TOkeshu's patch locally, and it didn't open the Help page. Looking at the code I realized his code would work from content but not from inside Chrome. Modified it to mirror the FxA login and the GettingStarted logic. Tested my mods on top of his patch locally and modified the tests to match. It works for me. Here's the try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=e1694546b5c0
Attachment #8527226 -
Flags: review?(standard8)
Comment 19•9 years ago
|
||
Comment on attachment 8527226 [details] [diff] [review] [PATCH] : Add a link to Hello's SUMO page Review of attachment 8527226 [details] [diff] [review]: ----------------------------------------------------------------- The original patch using window.open is actually correct - there's an issue with e10s windows that prevent the opened tab loading the page properly. Of the possible candidates for it, bug 1047603 looks most relevant, although there's also bug 958829 and bug 1037760. I don't think we should add this complexity unnecessarily.
Attachment #8527226 -
Flags: review?(standard8) → review-
Comment 20•9 years ago
|
||
(I tested a build with attachment 8526027 [details] [diff] [review] and activating help in e10s and non-e10s windows, I believe the ToS/privacy links also have the same issue.
Comment 21•9 years ago
|
||
I chatted to Maire about this over irc. We're going to separate out the e10s issue to a separate bug as it affects other links in the panel as well - we can then be explicit about what we're doing. This patch is the orginal attachment 8526027 [details] [diff] [review] with Maire's review comment fixes ported into it.
Attachment #8527226 -
Attachment is obsolete: true
Attachment #8527558 -
Flags: review+
Comment 22•9 years ago
|
||
I've split out the e10s issue into bug 1103897.
Comment 23•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/5b8efb0b1f9b
Assignee: mreavy → rgauthier
Iteration: --- → 36.3
Points: --- → 2
Target Milestone: mozilla35 → mozilla36
https://hg.mozilla.org/mozilla-central/rev/5b8efb0b1f9b
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 25•9 years ago
|
||
Comment on attachment 8527558 [details] [diff] [review] Add a link to Loop's help page in the gear menu. Approval Request Comment [Feature/regressing bug #]: N/A [Risks and why]: It's important to be able to get to the Sumo support page easily. [String/UUID change made/needed]: none
Attachment #8527558 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8527558 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•9 years ago
|
status-firefox35:
--- → fixed
status-firefox36:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•