GNOME Bugzilla – Bug 382517
HIG fixes for new color tab
Last modified: 2007-11-09 17:30:57 UTC
I am attaching two patches for the new color tab. The first proposed patch fixes basic HIG issues with the color tab. The second proposed patch is my recommended redesigning of the color tab.
Created attachment 77710 [details] [review] Proposed Patch (Basic Fixes) 2006-12-04 Dennis Cranston <dennis_cranston@yahoo.com> * theme-properties.glade: Basic HIG fixes for color tab. Add a mnemonic to the check button. Change tab label to "Color". Add colons to row labels. Fix widget spacing.
Created attachment 77711 [details] [review] Proposed Patch (Redesign) 2006-12-04 Dennis Cranston <dennis_cranston@yahoo.com> * theme-properties.glade: HIG fixes for color tab. Add a mnemonic to the check button. Normal, input, and selected color redesign. Change tab label to "Color".
Created attachment 77712 [details] Screenshot of proposed redesign
OK, all looks good to me.
Could you commit then, please?
Are you asking me or Thomas to commit?
I'm currently wondering whether the text colour options are really needed. It should be possible to pick either black or what based on the background colour. Any thoughts?
I hate to answer your question with a question, but what if a user does not want black text?
Hmm... FWIW, at first glance I think I actually prefer the original, more compact layout, although it does make it impossible to provide a mnemonic for every colour chip :/ But I don't think it would be too bad just to have mnemonics for Normal, Input and Selection, and just have to use Tab to get to the second chip in each case. BTW, even though this dialog is instant apply, I think we really ought to have some sort of preview for each of the three categories as well... you might not necessarily have an input box or selected text visible while you're doing this, for example. That leads on to the question of whether explicit apply would be more sensible for this tab. I think that relates to comments 7 & 8, i.e. whether we think it's a good idea to let users pick their own normal text colour, or always just choose a contrasting one for them-- if we always picked a contrasting one they couldn't screw up badly enough to make their desktop unusable, so explicit apply wouldn't really be necessary. On the other hand, we'd probably get complaints about infringement of chromatic freedom if we took away the text colour options, and I guess keeping them might also get us one step closer to obsoleting (or at least simplifying) the Low Contrast themes, which currently use grey text on a grey background.
Created attachment 77903 [details] [review] Proposed Patch (Basic Fixes with Mnemonics) This patch just adds mnemonics for normal, input, and selection as suggested by Calum. But, it is not a simple matter of just using Tab to get to the second color-button. Selecting the mnemonic will activate the first color-button, so the user will have to cancel the color selection dialog and then Tab to the second color-button.
Created attachment 77912 [details] Screenshot of Color Tab with a Preview I reworked the Color tab again to include a preview section. This is a screen-shot of those changes. Comments are welcome. Thanks.
OK, but does this mean we need to make the dialog explicit apply?
Just a couple of ideas from the peanut gallery: If this is to be an instant apply window it might be nice to have a 'revert' button. One thing I have seen in similar UIs is that if the user picks the same color for BG and FG, the FG gets dimmed slightly so you can read it - it basically won't let you pick the same color. Some smarts like that here might make sense especially if it's to be instant apply. A big risk here is folks picking the same color and then not knowing how to fix it because they can't read any of the widgets anymore. o_O Another idea is instead of having the single color swatch buttons to pull up the color chooser, have foreground/background buttons as gaim does for selecting font foreground/background colors (see screenshot: http://gaim.sourceforge.net/images/screenshots/wysiwyg.png) Hope this helps or at least gives some food for thought.
Comments on the reworked patch/screenshot: - I'd say "Enable custom colours" probably doesn't need to be in bold; bold just looks a little odd in anything other than a regular frame heading (even it's doubling up as a frame heading here, it's a checkbox first and foremost). - The preview section looks really good, although I'd maybe try putting the controls in an embossed frame or something to emphasise that these aren't 'real' controls that are going to do anything. Could get a bit ugly though, I guess. - I'd agree the keynav behaviour described in #10 is troublesome. Is there any reasonable way to force the mnemonic just to focus the first button, rather than activate it? (E.g. have a hidden label for the second button with the same mnemonic... or is gtk+ clever enough to ignore mnemonics for labels that are hidden?) - Now that the Preview area is there, it doesn't really seem right for these changes to be instant apply, I don't think (even though I mused earlier that it might still work). I suspect we should just have an "Apply" button somewhere in or around the Preview area.
I just committed some of the suggestions from this bug. I also had new names for the different colours, which are hopefully more obvious to new users: Normal -> Windows Input -> Input Boxes Selected -> Selected Items I wasn't sure how the mnemonics where chosen in the previous examples, but I looked in the HIG and it suggested using the first letter of the label if possible, so this is what I did.
One comment in regards to the applied changes. Capitalization is not right. "Enable Custom Colors" should be "Enable custom colors". Also, "Input Boxes" and "Selected Items" should be "Input boxes" and "Selected items".
One other note, the mnemonic for the "Customize..." button conflicts with the mnemonic of the close button.
I have now fixed the problems mentioned in the last two comments. I'm still looking for suggestions about the layout of an explicit apply tab (such as where the apply button should be).
Marking patches as reviewed since most of the changes (apart from the preview function) is now in.
Confirming bug too, as we obviously do want HIG fixes!
Is there anything left to do here?
Assuming fixed.