Closed
Bug 591753
Opened 14 years ago
Closed 9 years ago
Category buttons should highlight on hover even when selected
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla43
People
(Reporter: martijn.martijn, Assigned: 15electronicmotor)
Details
(Whiteboard: [good first bug])
Attachments
(2 files, 3 obsolete files)
963 bytes,
patch
|
mossop
:
review-
|
Details | Diff | Splinter Review |
760 bytes,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
The mouse cursor of the left column entries (Get Add-ons, Extension, Themes, Plugins) should be a pointer, so I know that when clicking on them, something happens. When I'm under "More" under extensions, for example, I'm not really sure I could click on "Extensions" again, to get back again to the beginning of "Extensions". And when you're at the beginning of "Extensions", the cursor of the "Extensions" entry should be normal, then.
Comment 1•14 years ago
|
||
Makes sense. Should be an easy fix.
Severity: normal → minor
blocking2.0: --- → ?
OS: Windows 7 → All
Hardware: x86 → All
Whiteboard: [good first bug]
Version: unspecified → Trunk
Comment 2•14 years ago
|
||
I'm not sure we should do this but if UX agree then I'd take a patch. Doesn't block though.
blocking2.0: ? → -
Comment 3•13 years ago
|
||
Not sure we'd want this, tbh. The cursor isn't a "hand" on normal browser tabs, so I don't see why it would be for these items (which are essentially tabs rotated 90 deg). However, I think it might make sense to give them hover effects, depending on the platform. Still uiwanted for that, though. :)
Reporter | ||
Comment 4•13 years ago
|
||
With tabs you at least have a hover effect, the left column entries in the addons manager only have a useless tooltip if you stay too long.
Comment 5•11 years ago
|
||
Jennifer, are you still working on this? I'm going through older "good first bugs" that have been inactive for a while. Is this still valid?
Flags: needinfo?(jboriss)
Assignee | ||
Comment 6•9 years ago
|
||
I would like to work on this bug.Please assign it to me if nobody working on it.
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8630393 -
Flags: review?(robert.strong.bugs)
Attachment #8630393 -
Flags: feedback?
Comment 8•9 years ago
|
||
Comment on attachment 8630393 [details] [diff] [review] bug_591753_1.diff This really should be reviewed by Dave Townsend or Blair McBride or be directed to someone that has time and the knowledge to review this code even though they are both marked as not accepting review requests
Attachment #8630393 -
Flags: review?(robert.strong.bugs)
Attachment #8630393 -
Flags: feedback?
Comment 9•9 years ago
|
||
Dave, can either you or Blair either review this or direct this towards someone for review
Flags: needinfo?(dtownsend)
Comment 10•9 years ago
|
||
Comment on attachment 8630393 [details] [diff] [review] bug_591753_1.diff Review of attachment 8630393 [details] [diff] [review]: ----------------------------------------------------------------- I can review it, but I still want UX to weigh in on whether this is the right thing. Markus? ::: toolkit/mozapps/extensions/content/extensions.css @@ +24,5 @@ > +.category[selected="true"] > * { > + cursor: default; > +} > + > +ort-controls { This looks broken
Attachment #8630393 -
Flags: feedback?(mjaritz)
Updated•9 years ago
|
Flags: needinfo?(jboriss)
Flags: needinfo?(dtownsend)
Comment 11•9 years ago
|
||
Dave, thanks for bringing me in. We should keep aligned with about:preferences on this. As we treat inContent UI as all UI we should not use the hand-cursor. Having a hover state is enough indicator that this UI element is interactive. (We only use the Hand-Cursor on text-links that do look like links. e.g. More Information; other UI elements work with a hover effect e.g. buttons, lists )
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Comment 12•9 years ago
|
||
Comment on attachment 8630393 [details] [diff] [review] bug_591753_1.diff Review of attachment 8630393 [details] [diff] [review]: ----------------------------------------------------------------- please do not implement hand-cursor for left column entries.
Attachment #8630393 -
Flags: feedback?(mjaritz) → feedback-
Assignee | ||
Comment 13•9 years ago
|
||
Sorry for late response and thanks for review. If I don't implement hand-cursor then how users know their is something more. Maybe you can tell me what to use.
Flags: needinfo?(mjaritz)
Comment 14•9 years ago
|
||
These buttons currently have a hover-state - the background-color changes on hover. This is enough indication that there is something more. So there is nothing more to do on this bug. Thanks!
Flags: needinfo?(mjaritz)
Comment 15•9 years ago
|
||
Sorry shivang. In my reply, Imissed one aspect of this bug. (In reply to Martijn Wargers [:mwargers] (QA) from comment #0) > When I'm under "More" under extensions, for example, I'm not really sure I > could click on "Extensions" again, to get back again to the beginning of > "Extensions". We should make sure to have a hover effect on the select element as well. Best only if the user is one level down (e.g. under "More" in Extensions) This hover should be similar to the hover of other buttons, but a bit darker. I would suggest using #4F5961 as a hover of the active element. Thank you Shivang.
Assignee: jboriss → 15electronicmotor
Status: RESOLVED → REOPENED
Flags: needinfo?(15electronicmotor)
Resolution: WONTFIX → ---
Assignee | ||
Comment 16•9 years ago
|
||
I changed the respective css file according to you suggestion but I really think there is not need of more hovering effect on selected button.
Flags: needinfo?(15electronicmotor)
Assignee | ||
Comment 17•9 years ago
|
||
Attachment #8642589 -
Flags: review?(mjaritz)
Attachment #8642589 -
Flags: feedback?(mjaritz)
Comment 18•9 years ago
|
||
Comment on attachment 8642589 [details] [diff] [review] Added more hovering effect Hello shivang, this left column entries should not have a hand cursor, but only the hover effect they currently have. I was suggesting adding a hover effect for the selected button for users to see that it is clickable, even if selected. (This is important for when "users" are on level down, e,g, under "more", as you already mentioned.) Seeing the color I suggested implemented, I realize this is the wrong color. Having the dark selected state become darker (#1A2533) on hover keeps better aligned with the design, while still providing a hover. We should add only the css for hover, and no hand-cursor: .category[selected="true"]:hover { background-color:#1A2533; }
Attachment #8642589 -
Flags: review?(mjaritz)
Attachment #8642589 -
Flags: review-
Attachment #8642589 -
Flags: feedback?(mjaritz)
Attachment #8642589 -
Flags: feedback+
Assignee | ||
Comment 19•9 years ago
|
||
Attachment #8630393 -
Attachment is obsolete: true
Attachment #8642589 -
Attachment is obsolete: true
Attachment #8643008 -
Flags: review?(mjaritz)
Attachment #8643008 -
Flags: feedback?(mjaritz)
Comment 20•9 years ago
|
||
Comment on attachment 8643008 [details] [diff] [review] bug_591753_3.diff UI-wise it is ok. For review, best ask Dave [:mossop]
Attachment #8643008 -
Flags: review?(mjaritz)
Attachment #8643008 -
Flags: review?(dtownsend)
Attachment #8643008 -
Flags: feedback?(mjaritz)
Attachment #8643008 -
Flags: feedback+
Comment 21•9 years ago
|
||
Comment on attachment 8643008 [details] [diff] [review] bug_591753_3.diff Review of attachment 8643008 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/mozapps/extensions/content/extensions.css @@ +20,5 @@ > +.category[selected="true"]:hover { > + background-color:#1A2533; > +} > + > +ort-controls { This looks broken, can you correct it and resubmit?
Attachment #8643008 -
Flags: review?(dtownsend)
Assignee | ||
Comment 22•9 years ago
|
||
Sorry about last patch.
Attachment #8643008 -
Attachment is obsolete: true
Attachment #8644398 -
Flags: review?(dtownsend)
Attachment #8644398 -
Flags: feedback?(dtownsend)
Updated•9 years ago
|
Summary: Mouse cursor of left column entries should be pointer → Category buttons should highlight on hover even when selected
Comment 23•9 years ago
|
||
Comment on attachment 8644398 [details] [diff] [review] Removed buggy code. Review of attachment 8644398 [details] [diff] [review]: ----------------------------------------------------------------- Sorry I should have spotted this error in the first place. The stylesheet you're amending here controls behavioural features of the UI. For styling you'll want to change this stylesheet: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/extensions/extensions.inc.css
Attachment #8644398 -
Flags: review?(dtownsend)
Attachment #8644398 -
Flags: review-
Attachment #8644398 -
Flags: feedback?(dtownsend)
Assignee | ||
Comment 24•9 years ago
|
||
But https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/extensions/extensions.inc.css is not the stylesheet for that Add-ons page, then why I have to change this stylesheet ?
Flags: needinfo?(dtownsend)
Comment 25•9 years ago
|
||
(In reply to shivang nagaria from comment #24) > But > https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/ > extensions/extensions.inc.css is not the stylesheet for that Add-ons page, > then why I have to change this stylesheet ? It is the stylesheet for the add-ons page, included through the platform specific stylesheets like https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/osx/mozapps/extensions/extensions.css#5
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 26•9 years ago
|
||
Attachment #8648469 -
Flags: review?(dtownsend)
Attachment #8648469 -
Flags: feedback?
Comment 27•9 years ago
|
||
Comment on attachment 8648469 [details] [diff] [review] edited appropriate css file. Review of attachment 8648469 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thank you for the patch.
Attachment #8648469 -
Flags: review?(dtownsend)
Attachment #8648469 -
Flags: review+
Attachment #8648469 -
Flags: feedback?
https://hg.mozilla.org/mozilla-central/rev/cf6138f13ae9
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•