146.34 KB, image/png
8.25 KB, image/png
60.67 KB, image/png
300.57 KB, patch
|Details | Diff | Splinter Review|
STR: Apply the patches for bug 582801, bug 667244, and bug 670304 (in that order). Build Thunderbird. See what's still wrong. ;) The purpose of this bug is to fix all the little things I find wrong with those other patches, since it seems like a real pain to try and untangle what bits each patch is responsible for, and where each individual fix goes. None of the patches in those bugs should be checked in until this patch is ready as well, and then we can do them all in one big swoop. Sound good? Thanks, Blake.
Now as the QFB button should always be visible, I think it would be better when the button becomes the state "disabled" instead of style="visibility: hidden;". This would made the CSS easier and made it saver the checked state can't be changed in disabled state.
(I've numbered these so that we can refer to them as, say "Aero-3", or "Mac-5".) Windows 7 Aero Problems: 1) There's a white border above the Quick Filter Button. 2) The QFB seems to always be a button, whereas the Lightning buttons are only button-y when you hover over them. (Okay, that's probably a problem with Lightning, but we should figure out what the fix is.) I dragged everything onto the toolbar at this point. 3) The border under the buttons is lighter than the border under the tabs. 4) Right-clicking in the tab bar area doesn't let me customize the buttons. 5) After moving everything off the toolbar, it got much smaller, and fell down behind the tabs. 6) (Probably unrelated) The "Get Mail" button isn't active in the Gloda Search Results page. 7) When the QFB is showing, the QF Button glows slightly brighter white, instead of glowing blue like the other buttons do (and like it does when the QFB isn't showing). (I think it should act like the "Tag" button.) And that's all I saw on Windows 7.
Created attachment 565002 [details] [diff] [review] All the patches from the sub-bugs, rolled up into one easy-to-apply patch. Hopefully the sub-bugs won't change their patches, and will just build on top of this one instead.
Windows 7 High-Contrast Black (a.k.a. "WinBlack"): 1) The backgrounds to the buttons are all grey-ish, and make the icons really hard to see. Ideally I think we'ld skip the gradient in this mode. (We might want to talk to the Gecko people about how to tell when we're in high-contrast mode...) I think that's it for High-Contrast.
Just a note that (5) from comment 2 is also appearing on Ubuntu Linux - the toolbar appears to be mostly hidden behind the tabs.
Created attachment 565039 [details] Customizing the toolbar on Ubuntu Linux Ok, in Ubuntu Linux still: 1) I'm also seeing the white border around the Quick Filter Button, when I'm customizing my toolbar. 2) Also seeing that the border under the buttons, when customizing, is just slightly darker. Screenshot attached.
Mac. Mostly good, but: 1) When dragging the buttons off the toolbar, I see the same behaviour as noted in Aero-5, even though the spinner is still on the toolbar. 2) After dragging everything off the toolbar, there's a light grey line at the bottom of the tab bar. <http://dl.dropbox.com/u/2301433/Screenshots/GreyLine.png> 3) There's a line at the left of the Quick Filter Button. I would expect that line to be at the end of the toolbar instead. 4) The Quick Filter Button depresses differently than the other buttons in the tab bar. (Yeah, it's the same as the Tab dropdown, but since we can now have other buttons there I think it should be made similar to them. Or they should be made similar to it…) Thanks, Blake.
Aero-1: fixed in the upcoming patch Aero-2: this will probably be fixed by bug 673990 Aero-3: not totally sure about this yet Aero-4: this is fairly complicated: the current implementation makes it hard to add a context menu to the "background" of the tab bar Aero-5: this is bug 687730 Aero-6: unrelated; I see this without the patch too (and it will probably be irrelevant once we switch to tabs-on-top anyway) Aero-7: I'll let Richard Marti chime in on this Linux-1: currently, we use a different image for the QFB when customizing. Maybe we want a colorful icon instead? Lunux-2: maybe we should just do away with the grey background when customizing; I think that's what's causing the issue, and Firefox doesn't do that (maybe we should do the same for the message header toolbar as well) Mac-1: bug 687730, again Mac-2: probably also bug 687730 Mac-3: not sure about this Mac-4: not sure about this
Created attachment 567319 [details] [diff] [review] Fix some issues with borders This should fix some issues with borders being screwy (notably the border on the tab bar toolbar and a bogus border on the left of the QFB itself under XP).
Attachment #565002 - Attachment is obsolete: true
(In reply to Jim Porter (:squib) from comment #8) > Lunux-2: maybe we should just do away with the grey background when > customizing; I think > that's what's causing the issue, and Firefox doesn't do that (maybe > we should do > the same for the message header toolbar as well) Without grey background you don't see any drop target. The whole Firefox tab bar is customizable. On TB only the tabbar-toolbar (a small part of the tab bar). If the tabbar-toolbar during customizing could expand to the last tab could make the need of the grey background obsolete.
Created attachment 567339 [details] [diff] [review] Fix more issues Patch including squib's fixes. Aero-3: fixed with this patch Aero-7: fixed with this patch. The QFB button used a state which is also defined for all the other buttons but was hard to trigger. So removed this state for the QFB button and let him glow blue. Mac-3: removed the border to more look as a part of the tab bar and also as a preparation for tabs on top where this borders are looking out of place. Mac-4: the QFB button is now acting always like a toolbarbutton (no hover state and the same background color when checked).
Attachment #567319 - Attachment is obsolete: true
Summarizing: Aero-1: fixed Aero-2: probably be fixed by bug 673990 Aero-3: fixed Aero-4: this is fairly complicated: the current implementation makes it hard to add a context menu to the "background" of the tab bar Aero-5: bug 687730 Aero-6: unrelated; Aero-7: fixed Mac-1: bug 687730, again Mac-2: probably also bug 687730 Mac-3: fixed Mac-4: fixed Linux-1: currently, we use a different image for the QFB when customizing. Maybe we want a colorful icon instead? Linux-2: maybe we should just do away with the grey background when customizing; I think that's what's causing the issue, and Firefox doesn't do that (maybe we should do the same for the message header toolbar as well) So, I think I'ld be okay with landing this if we add a more colourful icon for the QFB for when it's in the customization window and toolbar; do away with the grey background when customizing; and file a new bug for Aero-4. (Then, after that, hopefully we can land tabs-on-top, and life will be pretty good… ;) Paenglab, Squib, which one of you wants to take those last three items, and polish them off? Thanks, Blake.
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #12) > So, I think I'ld be okay with landing this if we add a more colourful icon > for the QFB for when it's in the customization window and toolbar; do away > with the grey background when customizing; and file a new bug for Aero-4. > (Then, after that, hopefully we can land tabs-on-top, and life will be > pretty good… ;) > > Paenglab, Squib, which one of you wants to take those last three items, and > polish them off? I can make the first, but I need the colorful icons. Could Andreas create them? And as own images or append to toolbar.png? I'm for the latter. As I stated in comment 10 without the grey background it would be hard to find the drop targen when the tabbar-toolbar is empty. If you want to remove it anyway, I can do it. The third should someone do who has better knowledge.
Hey all, just a heads up: "Aero-4: this is fairly complicated: the current implementation makes it hard to add a context menu to the "background" of the tab bar" I've been able to solve this with my tabs-on-top patch - right clicking on the tabs bar now shows the context menu. I'm also going to try to make it so that dragging toolbar items onto the tabs bar will cause those toolbar items to be dropped into the new toolbar to the right of the tabs bar. I'll keep you posted.
Ok, I've got dropping toolbar items onto the tabs bar working now - it chucks the toolbar items at the head of the tabbar-toolbar toolbar. This will be available in my patch for bug 644169 once I update it.
Since Mike got the context menu and toolbar item dropping working, I think we can remove the grey background. :) Andreas should be able to create the images (from comment 13) pretty quickly. And then we can check this in, along with the bugs it depends on… :)
(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #16) > Since Mike got the context menu and toolbar item dropping working, I think > we can remove the grey background. :) > > Andreas should be able to create the images (from comment 13) pretty quickly. Coming right up!
Created attachment 569641 [details] [diff] [review] graphics part of the icons Here are the graphics for XP, Aero and Pinstripe. For Linux I think we can use the search icon from GTK+ Will put up a more proper patch once I've got all the other patches this bug depends on running properly.
Created attachment 569772 [details] [diff] [review] graphics part of the icons part2 Richard noted a bit of overlap in mail-toolbar-small.png. Fixes that.
Created attachment 570001 [details] [diff] [review] Patch with colored QFB icons This patch removes the grey background on tabbar-toolbar in customizing mode. I'm using now always the colored icons from Andreas or system icons under Linux. I think it's the best always use this icons because the QFB button is now like a normal toolbarbutton and is placeable everywhere on the toolbars and should look always the same. I'm asking r and ui-r because now all issues should be solved and Blake said he wants to land shortly.
Created attachment 570003 [details] QFB button on Mac One thing I'm not sure is the Mac search icon. For me it looks a little bit taller (or a little shifted up). What do you mean, I took it between two of the tallest icons?
Yeah, it's definitely a pixel or two taller than the other icons. Andreas, did you want to shrink it down a tiny bit?
Comment on attachment 570001 [details] [diff] [review] Patch with colored QFB icons Review of attachment 570001 [details] [diff] [review]: ----------------------------------------------------------------- I don't really like the way the QFB icon changes on OS X (as demonstrated here: http://dl.dropbox.com/u/2301433/Screenshots/SpyglassStates.png ), particularly having the huge white glass bit disappear on hover feels really odd to me, and different from anything else. Also, it looks like we're cutting off the bottom of the shadow on the QFB icon. So I'm going to say ui-r-. Aside from that, the code seems pretty good, so r=me.
Some update about the QFB icon under OSX?
Created attachment 576213 [details] [diff] [review] Unbitrotted patch with colored QFB icons -[r+ and ui-r- by bwinton] I hope I'm not uploading out of turn here - bug 644169 needed this un-bitrotted, so I took the liberty of doing that. Nothing too major - but maybe go through it once to make sure I didn't do something silly.
Attachment #570001 - Attachment is obsolete: true
Created attachment 576470 [details] [diff] [review] updated version of the patch above This adds makes it :hover:active instead of :hover in quickFilterBar.css, so that the icons goes dark on downpress. Also removes the shadow on the search icon.
Created attachment 576508 [details] graphical glitch with search button The search icon seems to become a archive when any other tab than the main one is selected. Not quite sure why that is happening yet.
Created attachment 576515 [details] [diff] [review] Next try for review This patch fixes the glitch on inactive QFB button on Mac (wrong -moz-image-region definition). I'm asking only for ui-r because since the r+ only the unbitrot and my glitch fix have changed. But feel free if you want change it.
Created attachment 576548 [details] [diff] [review] Again a try for review I've found the unbitrot patch corrupted the files mainwindow-dropdown-arrow-inverted.png and tab-arrow-left-inverted.png. This patch corrects this.
Works much better now! A small thing I noted when dragging it around was that I don't seem get any icon for Search in the customize toolbar window.
and this seems to be because it get visibility: hidden; added from somewhere.
Created attachment 576611 [details] [diff] [review] Last try for review? I could reproduce the inactive icon state. This happens only when the QFB button is moved to the customize window on a tab where the button is in disabled state. Now the button is always visible in customize window.
Comment on attachment 576611 [details] [diff] [review] Last try for review? Review of attachment 576611 [details] [diff] [review]: ----------------------------------------------------------------- So, it still looks really odd to me, and so I've spent some time thinking about why. 1) While all the other toolbar buttons go dark on click, they all have more blocky shapes, and are multi-coloured to start off with, so the darkening looks much better than the spyglass where only the circle in the middle is changed. 2) In Firefox, when you click the monochrome buttons in the toolbar either the shadow underneath them gets darker, or they glow blue, and this is the direction I'ld like to see us head. But this set of patches has been lingering for far too long, so I think I'm okay with landing them, and doing the rest of the work in a follow-up bug. Thanks, Blake.
Attachment #576611 - Flags: ui-review?(bwinton) → ui-review+
Asking checkin-needed so the follow-up bugs can be filed ;).
Checked in to comm-central as http://hg.mozilla.org/comm-central/rev/24a39254d6a8
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Version: unspecified → 11
Assignee: nobody → richard.marti
Target Milestone: --- → Thunderbird 11.0
You need to log in before you can comment on or make changes to this bug.