Last Comment Bug 844432 - Give Edit buttons a hover state
: Give Edit buttons a hover state
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: 17 Branch
: All Mac OS X
: -- normal (vote)
: Thunderbird 22.0
Assigned To: Richard Marti (:Paenglab)
:
:
Mentors:
Depends on: 792021
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-23 02:29 PST by Richard Marti (:Paenglab)
Modified: 2013-03-28 04:59 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
20+
fixed


Attachments
proposed fix (3.71 KB, patch)
2013-02-23 02:36 PST, Richard Marti (:Paenglab)
bwinton: review+
bwinton: ui‑review+
Details | Diff | Splinter Review
patch in action (7.37 KB, image/png)
2013-02-23 02:37 PST, Richard Marti (:Paenglab)
no flags Details
proposed fix v2 (3.75 KB, patch)
2013-02-24 04:10 PST, Richard Marti (:Paenglab)
bwinton: review+
bwinton: ui‑review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
standard8: approval‑comm‑esr17+
Details | Diff | Splinter Review
patch v2 in action (9.88 KB, image/png)
2013-02-24 04:13 PST, Richard Marti (:Paenglab)
no flags Details

Description Richard Marti (:Paenglab) 2013-02-23 02:29:01 PST
+++ This bug was initially created as a clone of Bug #792021 +++

Comment on attachment 709778 [details] [diff] [review] [diff] [review]
patch v2

Okay, ui-r=me, but I'ld like you to file a couple of followup bugs, if you don't mind.
1) The cut/copy/paste icons don't have a hover state, and I think they should act more like the ▶ icon.
Comment 1 Richard Marti (:Paenglab) 2013-02-23 02:36:58 PST
Created attachment 717489 [details] [diff] [review]
proposed fix

Blake, I take you for reviews because you wanted this and I think it's the best you check this.

I've added white icons when in hover state. Additionally I removed the margins around the icons to make the edit menu only 18px tall (minimal icon height) and not 22px as before. the original menus are 17px tall.
Comment 2 Richard Marti (:Paenglab) 2013-02-23 02:37:47 PST
Created attachment 717490 [details]
patch in action

Screenshot for easier review
Comment 3 Blake Winton (:bwinton) (:☕️) 2013-02-23 17:46:27 PST
Comment on attachment 717489 [details] [diff] [review]
proposed fix

While I appreciate that the background on hover is now the same height as the disclosure arrows, the cut/copy/paste icons seem to be pushing up against the edges a little now…  Is there any way you could make smaller icons for those?  I suspect the answer might be "No", so I'm going to say ui-r=me, even with the larger icons, so that this can move forward, but if you could make smaller icons, I think that would look nicer…

And the code seems fine, so r=me, as well.

Thanks,
Blake.
Comment 4 Richard Marti (:Paenglab) 2013-02-24 04:10:11 PST
Created attachment 717611 [details] [diff] [review]
proposed fix v2

My bad, I haven't realised the icons where already 16px (the glyphs are 14px with 1px around them) and drawn as 18px through the .toolbarbutton-icon rule. This patches fixes it to 16px with 1px margin. This lets this menu 18px tall but gives 2px space for the highlight background.

What do you think, is this a candidate to land on ESR because it fixes also the wrong icon dimensions?
Comment 5 Richard Marti (:Paenglab) 2013-02-24 04:13:58 PST
Created attachment 717613 [details]
patch v2 in action

I don't mark attachment 717490 [details] as obsolete for a better comparison of the space around the glyphs.
Comment 6 Blake Winton (:bwinton) (:☕️) 2013-02-24 10:40:46 PST
Comment on attachment 717611 [details] [diff] [review]
proposed fix v2

r=me, ui-r=me!

I'm not sure about landing it in the ESR…  Hopefully Standard8 or Irving will weigh in.

Thanks,
Blake.
Comment 7 Ryan VanderMeulen [:RyanVM] 2013-02-25 06:51:46 PST
https://hg.mozilla.org/comm-central/rev/3e82a183e342
Comment 8 Mark Banner (:standard8) 2013-03-05 03:29:18 PST
Comment on attachment 717611 [details] [diff] [review]
proposed fix v2

[Triage Comment]
I think we possibly could take this on esr, but lets get some testing on aurora/beta first.
Comment 9 Mark Banner (:standard8) 2013-03-05 06:39:20 PST
https://hg.mozilla.org/releases/comm-aurora/rev/206010efb93f
Comment 10 Mark Banner (:standard8) 2013-03-05 06:51:47 PST
https://hg.mozilla.org/releases/comm-beta/rev/36d033ee68cb
Comment 11 Mark Banner (:standard8) 2013-03-28 04:59:32 PDT
https://hg.mozilla.org/releases/comm-esr17/rev/57e2f985b4bd

Note You need to log in before you can comment on or make changes to this bug.