Closed
Bug 615318
Opened 14 years ago
Closed 14 years ago
Add a "not now" choice to all doorhanger notification split buttons
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: faaborg, Assigned: Margaret)
References
(Blocks 1 open bug)
Details
(Keywords: late-l10n, Whiteboard: [strings][target-betaN][doorhanger])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
We would like to teach users that doorhanger notifications are ignorable, and that dismissing them is actually the equivalent of "not now" (as opposed to "no"). We should add a "Not now" command to the split button of every doorhanger (even if it otherwise only has a single command), which contains explanatory text in the spot that usually contains keyboard shortcuts:
[action][v]
Action
---------------------------------
Not now Click outside to close
This way users can both have access to a way of specifically indicating not now, and in the process they can also pick up the message that it is actually more efficient to simply ignore the message.
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → margaret.leibovic
Comment 1•14 years ago
|
||
I think having a separate "not now" button rather than adding it to the menu would be much more discoverable. Some less savvy users may not know to view the menu and only see the split button as a single button. In this instance I would relegate the "Click outside to close" to the tooltip, as that information isn't as directly necessary if the user does have a direct "not now" button.
Reporter | ||
Comment 2•14 years ago
|
||
Note that we are going to use this our our only approach for "not now" to see how well it works. Worst case scenario we move forward with bug 615315 as well.
Reporter | ||
Comment 3•14 years ago
|
||
>I think having a separate "not now" button rather than adding it to the menu
>would be much more discoverable.
If we present the user with two buttons, they are going to interpret it as a forced choice modal dialog (even though it isn't actually forced choice, and it isn't actually modal). Also, they are more likely to think "I should actually read and decide what I want to do with this" when we are actually hoping that they ignore the prompt in certain situations where the user wasn't expecting it (random forum web site would like to know your physical location, etc.)
Assignee | ||
Comment 4•14 years ago
|
||
This is going to require updating the tests in browser_popupNotification.js and get approval for string changes. I'll let Alex fight for the strings if this is really what we want to do ;)
Attachment #493858 -
Flags: feedback?(gavin.sharp)
Reporter | ||
Comment 5•14 years ago
|
||
>[Strings]
UI change based on the feedback coming in. It's not ideal that we have to break string freeze to add this, but I think it does improve the usability of the control.
blocking2.0: --- → ?
Whiteboard: [strings]
Comment 6•14 years ago
|
||
(In reply to comment #3)
I see your point. The problem is "click outside to close" is an alien concept to the bulk of users and unfortunately even split buttons are so to many as well. Most users will probably need bug 615315 in some form. (see also bug 615315 comment 2)
It might be good to have both, such that if the user does select the menu they see the explicit choice to ignore always being available.
Comment 7•14 years ago
|
||
Comment on attachment 493858 [details] [diff] [review]
WIP
Did you consider adding the item to notification.xml#popup-notification's anonymous content, rather than adding it dynamically? I think that might be simpler.
That said, I'm not really happy about forcing this on all popup-notifications. It places some weird constraints on the general notification API and probably isn't something we'd be able to undo later. I'm also not sure it would be a very effective teaching aid... Explanatory-text-in-accelText just seems kind of strange, and it seems to me that users confused by the interaction model are likely to also be unfamiliar with menu-buttons, which would hurt discoverability. I'd be much more comfortable with bug 615315, which I don't think suffers from any of those problems.
Attachment #493858 -
Flags: feedback?(gavin.sharp) → feedback-
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7)
> Comment on attachment 493858 [details] [diff] [review]
> WIP
>
> Did you consider adding the item to notification.xml#popup-notification's
> anonymous content, rather than adding it dynamically? I think that might be
> simpler.
I agree that sounds like a better idea. I'm not sure why I decided not to do that.
> That said, I'm not really happy about forcing this on all popup-notifications.
> It places some weird constraints on the general notification API and probably
> isn't something we'd be able to undo later. I'm also not sure it would be a
> very effective teaching aid... Explanatory-text-in-accelText just seems kind of
> strange, and it seems to me that users confused by the interaction model are
> likely to also be unfamiliar with menu-buttons, which would hurt
> discoverability. I'd be much more comfortable with bug 615315, which I don't
> think suffers from any of those problems.
The mockup in Alex's bug tracker mockup (http://people.mozilla.com/~faaborg/files/firefox4Mockups/polishNotifications-i1/polishNotifications-i1.htm) adds more to the menu popup than my patch, and I think that might make the user's options clearer. I don't think it will be too hard to make a patch, so maybe this is one of those things that we can get in front of nightly users and see what they think?
Assignee | ||
Comment 9•14 years ago
|
||
This patch adds the close menuitem to the binding. However, the dynamic menuitems are being inserted after it when the notification is re-opened (they are inserted before it the first time the popup appears). Do you know what xbl issue would cause this?
Attachment #493858 -
Attachment is obsolete: true
Attachment #494046 -
Flags: feedback?(gavin.sharp)
Assignee | ||
Comment 10•14 years ago
|
||
I haven't tested how this looks on Linux yet.
Alex, do we want the "x" icon in the menuitem on Linux? I didn't include it on OSX, since other bugs have talked about how there should not be icons in menuitems on OSX.
Attachment #494046 -
Attachment is obsolete: true
Attachment #494889 -
Flags: review?(gavin.sharp)
Attachment #494046 -
Flags: feedback?(gavin.sharp)
Reporter | ||
Comment 11•14 years ago
|
||
Linux yes, OS X no.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [strings] → [strings][needs review gavin]
Assignee | ||
Comment 12•14 years ago
|
||
Oops, I used the wrong icon url in the gnomestripe css. However, I still can't get the icon to appear in the menu, and I'm not seeing other menuitem icons appear. Is there some pref that shows/hides menuitem icons on Linux?
Comment 13•14 years ago
|
||
(In reply to comment #12)
> Oops, I used the wrong icon url in the gnomestripe css. However, I still can't
> get the icon to appear in the menu, and I'm not seeing other menuitem icons
> appear. Is there some pref that shows/hides menuitem icons on Linux?
Yes, that is a gnome system pref. They are off by default. I think there is a checkbox under the Appearance Preferences > Interface tab.
Assignee | ||
Comment 14•14 years ago
|
||
I got rid of the color: graytext; style for the gnomestripe acceltext because it was impossible to read when highlighed. I could also set a [_moz-menuactive="true"] color style to make it readable, but I feel like Linux themes vary so much that we're best off if we just use the default system colors.
Attachment #494889 -
Attachment is obsolete: true
Attachment #495572 -
Flags: review?(gavin.sharp)
Attachment #494889 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 15•14 years ago
|
||
>I could also set a
>[_moz-menuactive="true"] color style to make it readable, but I feel like Linux
>themes vary so much that we're best off if we just use the default system
>colors.
Given the placement on a second line, it's going to look like the main command if we don't provide some way of de-emphasizing it. Let's go with the [_moz-menuactive="true"] approach.
Updated•14 years ago
|
Whiteboard: [strings][needs review gavin] → [strings][needs review gavin][target-betaN]
Updated•14 years ago
|
blocking2.0: ? → betaN+
Assignee | ||
Comment 16•14 years ago
|
||
Johnath, this has strings. Should it block beta9?
Updated•14 years ago
|
blocking2.0: betaN+ → beta9+
Assignee | ||
Comment 17•14 years ago
|
||
(In reply to comment #15)
> >I could also set a
> >[_moz-menuactive="true"] color style to make it readable, but I feel like Linux
> >themes vary so much that we're best off if we just use the default system
> >colors.
>
> Given the placement on a second line, it's going to look like the main command
> if we don't provide some way of de-emphasizing it. Let's go with the
> [_moz-menuactive="true"] approach.
What color should we do for the "Click Outside to Close" text when it's highlighted? Using -moz-menubarhovertext will guarantee it's readable, but then it will be the same color as the "Not Now" text.
Assignee | ||
Comment 18•14 years ago
|
||
After talking with Alex, we decided to get rid of the "Click Outside to Close" text.
Attachment #495572 -
Attachment is obsolete: true
Attachment #497923 -
Flags: review?(gavin.sharp)
Attachment #495572 -
Flags: review?(gavin.sharp)
Updated•14 years ago
|
Attachment #497923 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [strings][needs review gavin][target-betaN] → [strings][can land][target-betaN]
Assignee | ||
Comment 19•14 years ago
|
||
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [strings][can land][target-betaN] → [strings][target-betaN]
Comment 20•14 years ago
|
||
Sorry if this is the wrong place to discuss about this, but this makes it impossible to have notifications that are just used for informing about something that has happened. I mean notification with only an OK-button or something similar. The choices now are either to have an OK-button, with "Not now" -option (doesn't make much sense to "not now" on something that has already happened) OR no button's at all which means no way to dismiss the panel, unless user knows to click outside of it, and no way to completely remove the notification.
Comment 21•14 years ago
|
||
Tuomas: that's a good point. Can you file that as a separate bug, and CC me?
Comment 22•14 years ago
|
||
Done. Bug 622969
Comment 23•14 years ago
|
||
As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)
Comment 24•14 years ago
|
||
(In reply to comment #23)
Christian, you somehow wiped the entire CC list for this bug... fixing...
Updated•14 years ago
|
Whiteboard: [strings][target-betaN] → [strings][target-betaN][doorhanger]
Comment 25•13 years ago
|
||
Could this be the cause of the regression in bug 650247? The geolocation API no longer triggers the error handler when selecting "Now now" since Firefox 4. (Or a recent Aurora, for that matter. Build ID 20110704042003.)
Related: bug 591745.
You need to log in
before you can comment on or make changes to this bug.
Description
•