Last Comment Bug 783822 - [Mac default] adopt editBMPanel to new light coloring
: [Mac default] adopt editBMPanel to new light coloring
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: seamonkey2.15
Assigned To: Stefan [:stefanh]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-18 14:52 PDT by Stefan [:stefanh]
Modified: 2012-10-07 12:39 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
WIP patch (19.96 KB, patch)
2012-08-18 14:52 PDT, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Fixed colors (20.03 KB, patch)
2012-08-30 13:25 PDT, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Almost there... (21.74 KB, patch)
2012-09-02 12:14 PDT, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Final version (20.92 KB, patch)
2012-09-13 10:22 PDT, Stefan [:stefanh]
mnyromyr: review-
Details | Diff | Splinter Review
New version (20.92 KB, patch)
2012-10-03 10:34 PDT, Stefan [:stefanh]
mnyromyr: review+
iann_bugzilla: approval‑comm‑aurora+
Details | Diff | Splinter Review
Save dialog on 10.8 (91.80 KB, image/png)
2012-10-03 12:29 PDT, Stefan [:stefanh]
no flags Details

Description Stefan [:stefanh] 2012-08-18 14:52:35 PDT
Created attachment 653110 [details] [diff] [review]
WIP patch

Bug 771284 will change the way arrow panels look (dark --> light). We need to adjust the css for our editBMPanel in order to not make it look broken.

Attaching a WIP patch.
Comment 1 Stefan [:stefanh] 2012-08-19 09:14:25 PDT
Note to self:
* Fix colors (fff --> FFFFFF etc)
* Double-check focus border
Comment 2 Stefan [:stefanh] 2012-08-30 13:25:25 PDT
Created attachment 657011 [details] [diff] [review]
Fixed colors
Comment 3 Stefan [:stefanh] 2012-09-02 12:14:05 PDT
Created attachment 657698 [details] [diff] [review]
Almost there...

panel hack will be removed in final patch...
Comment 4 Stefan [:stefanh] 2012-09-02 12:20:33 PDT
2.14 will need this, since the panel change was made for mozilla 17.
Comment 5 Stefan [:stefanh] 2012-09-13 10:22:30 PDT
Created attachment 660885 [details] [diff] [review]
Final version

Some polish and clean-up, compared to the previous patch.
Comment 6 Karsten Düsterloh 2012-10-02 13:07:44 PDT
Comment on attachment 660885 [details] [diff] [review]
Final version

The light colour scheme is definitely an improvement — I never liked the dark stuff anyway. ;-)

>+#editBMPanel_rows > row > textbox[focused="true"],
>+#editBMPanel_rows > row > hbox > textbox[focused="true"] {
>+  border-color: -moz-mac-focusring !important;
>+  /*box-shadow: 0 0 1px -moz-mac-focusring inset,
>+              0 0 4px 1px -moz-mac-focusring,
>+              0 0 1.5px 1px -moz-mac-focusring; */
>+}

The focus rect around the "name" and "tags" fields does not match the one around all other elements, though, which looks very odd. I assume you just forgot to remove the /* */ above and it wasn't on purpose … ;-)

Plus, I wonder why you made the three buttons at the bottom that oblong? They look different now than almost everywhere else, not like "real" buttons …
Comment 7 Stefan [:stefanh] 2012-10-03 10:33:12 PDT
(In reply to Karsten Düsterloh from comment #6)
> Comment on attachment 660885 [details] [diff] [review]
> Final version
> 
> The light colour scheme is definitely an improvement — I never liked the
> dark stuff anyway. ;-)
> 
> >+#editBMPanel_rows > row > textbox[focused="true"],
> >+#editBMPanel_rows > row > hbox > textbox[focused="true"] {
> >+  border-color: -moz-mac-focusring !important;
> >+  /*box-shadow: 0 0 1px -moz-mac-focusring inset,
> >+              0 0 4px 1px -moz-mac-focusring,
> >+              0 0 1.5px 1px -moz-mac-focusring; */
> >+}
> 
> The focus rect around the "name" and "tags" fields does not match the one
> around all other elements, though, which looks very odd. I assume you just
> forgot to remove the /* */ above and it wasn't on purpose … ;-)
>

Ouch, sorry - I commented that out when testing if the !important was needed (yes, it was), then I must have forgotten it.

> Plus, I wonder why you made the three buttons at the bottom that oblong?
> They look different now than almost everywhere else, not like "real" buttons
> …

The min-width of 79px comes from button.css, I guess you could see them as panel "versions" of the normal buttons. Note that they look exactly the same as the ones in the  Firefox version of the panel. I see your point, but I would rather stay in parity with Firefox here (and hasn't Apple's buttons always looked a bit too wide if you don't have enough text in the label?)
Comment 8 Stefan [:stefanh] 2012-10-03 10:34:46 PDT
Created attachment 667537 [details] [diff] [review]
New version

See my previous comment.
Comment 9 Karsten Düsterloh 2012-10-03 11:45:00 PDT
(In reply to Stefan [:stefanh] from comment #7)
> > Plus, I wonder why you made the three buttons at the bottom that oblong?
> > They look different now than almost everywhere else, not like "real" buttons
> > …
> 
> The min-width of 79px comes from button.css, I guess you could see them as
> panel "versions" of the normal buttons.

I'm not concerned about their length but about their actual _form_. Usually, all(?) our buttons have normal Mac-like form with "half circles" on the left and right, but these don't. 

> Note that they look exactly the same as the ones in the Firefox version 
> of the panel.

Maybe.
I'd bet they didn't care about the form on Mac.

> I see your point

I'm not sure you do. ;-)
Comment 10 Stefan [:stefanh] 2012-10-03 12:29:29 PDT
Created attachment 667599 [details]
Save dialog on 10.8
Comment 11 Karsten Düsterloh 2012-10-03 13:30:13 PDT
Comment on attachment 667537 [details] [diff] [review]
New version

Ah, well, who am I to blame Apple of inconsistent design …
Comment 12 Stefan [:stefanh] 2012-10-03 14:01:09 PDT
http://hg.mozilla.org/comm-central/rev/9c30ed8bfa50
Comment 13 Stefan [:stefanh] 2012-10-03 14:36:12 PDT
Comment on attachment 667537 [details] [diff] [review]
New version

[Approval Request Comment]
Regression caused by (bug #): Bug 771284
User impact if declined: Odd-looking editBookmarkPanel (mixture of old/new styles)
Testing completed (on m-c, etc.): Nope
Risk to taking this patch (and alternatives if risky): No risk, only theme changes
String changes made by this patch: None

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