r92846 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92845‎ | r92846 | r92847 >
Date:02:11, 22 July 2011
Author:kaldari
Status:ok (Comments)
Tags:
Comment:
follow-up to r91499 - set padding for the elements inside the button rather than for the button itself - this is how normal jquery.UI skins handle it, as it prevents the button padding from screwing up the positioning of the icons.
Modified paths:
  • /trunk/phase3/resources/jquery.ui/themes/vector/jquery.ui.button.css (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery.ui/themes/vector/jquery.ui.button.css
——@@ -9,22 +9,22 @@
1010
1111 /*button text element */
1212 .ui-button .ui-button-text { display: block; line-height: 1.4em; }
13 -.ui-button-text-only .ui-button-text { padding: .125em .25em; }
14 -.ui-button-icon-only .ui-button-text, .ui-button-icons-only .ui-button-text { padding: .4em; text-indent: -9999999px; }
15 -.ui-button-text-icon-primary .ui-button-text { padding: 0.1em 0.8em 0.1em 1.9em; }
16 -.ui-button-text-icon-secondary .ui-button-text { padding: .1em 1.9em .1em 0.8em; }
17 -.ui-button-text-icons .ui-button-text { padding: 0.1em 1.9em 0.1em 1.9em; }
 13+.ui-button-text-only .ui-button-text { padding: 0.3em 1em 0.25em 1em; }
 14+.ui-button-icon-only .ui-button-text, .ui-button-icons-only .ui-button-text { padding: 0.3em; text-indent: -9999999px; }
 15+.ui-button-text-icon-primary .ui-button-text, .ui-button-text-icons .ui-button-text { padding: 0.3em 1em 0.25em 2.1em; }
 16+.ui-button-text-icon-secondary .ui-button-text, .ui-button-text-icons .ui-button-text { padding: 0.3em 2.1em 0.25em 1em; }
 17+.ui-button-text-icons .ui-button-text { padding-left: 2.1em; padding-right: 2.1em; }
1818 /* for older versions of jQuery UI */
19 -.ui-button-text-icon .ui-button-text { padding: 0.1em 0.8em 0.1em 1.9em; }
 19+.ui-button-text-icon .ui-button-text { padding: 0.3em 1em 0.3em 2.1em; }
2020
2121 /* no icon support for input elements, provide padding by default */
22 -input.ui-button { padding: .4em 1em; }
 22+input.ui-button { padding: 0.3em 1em; }
2323
2424 /*button icon element(s) */
2525 .ui-button-icon-only .ui-icon, .ui-button-text-icon-primary .ui-icon, .ui-button-text-icon-secondary .ui-icon, .ui-button-text-icons .ui-icon, .ui-button-text-icon .ui-icon, .ui-button-icons-only .ui-icon { position: absolute; top: 50%; margin-top: -9px; }
2626 .ui-button-icon-only .ui-icon { left: 50%; margin-left: -8px; }
27 -.ui-button-text-icon-primary .ui-button-icon-primary, .ui-button-text-icon .ui-button-icon-primary, .ui-button-text-icons .ui-button-icon-primary, .ui-button-icons-only .ui-button-icon-primary { left: 0; }
28 -.ui-button-text-icon-secondary .ui-button-icon-secondary, .ui-button-text-icon .ui-button-icon-secondary, .ui-button-text-icons .ui-button-icon-secondary, .ui-button-icons-only .ui-button-icon-secondary { right: 16px; }
 27+.ui-button-text-icon-primary .ui-button-icon-primary, .ui-button-text-icon .ui-button-icon-primary, .ui-button-text-icons .ui-button-icon-primary, .ui-button-icons-only .ui-button-icon-primary { left: 0.5em; }
 28+.ui-button-text-icon-secondary .ui-button-icon-secondary, .ui-button-text-icon .ui-button-icon-secondary, .ui-button-text-icons .ui-button-icon-secondary, .ui-button-icons-only .ui-button-icon-secondary { right: 0.5em; }
2929
3030 /*button sets*/
3131 .ui-buttonset { margin-right: 7px; }
——@@ -36,7 +36,7 @@
3737 body .ui-button {
3838 -moz-border-radius: 4px;
3939 -webkit-border-radius: 4px;
40 - padding: 0.2em 0.6em 0.15em !important;
 40+ border-radius: 4px;
4141 margin: 0.5em 0 0.5em 0.4em !important;
4242 border: 1px solid #a6a6a6 !important;
4343 /* @embed */

Follow-up revisions

RevisionCommit summaryAuthorDate
r949471.17wmf1: MFT r91499, r92846catrope21:13, 18 August 2011
r94971this extra padding isnt needed any more per r92846kaldari00:14, 19 August 2011
r102514REL1_18 MFT r92846, r93065, r93834, r94171reedy14:31, 9 November 2011
r1025881.18wmf1 MFT r92846, r98764, r99477reedy22:51, 9 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r91499another fix for old jQuery UI on livekaldari22:03, 5 July 2011

Comments

#Comment by Kaldari (talk | contribs)   21:21, 27 July 2011

Here are screenshots of the 3 different button with icon variations using the css above: File:Jquery buttons.png

#Comment by Jorm (WMF) (talk | contribs)   22:03, 27 July 2011

Just pasting what I sent in an email:


So, first, I absolutely love - *LOVE* - the idea of a "lock" icon on disabled buttons, and I'm going to get right the fuck on adding that to the style guide.

That said, can we switch the locations of the icons?

When we talk about actions that move the user "forward" (e.g., preview, next, etc.) the icon should be on the right side. When we go "backwards" (cancel, etc.), it should be on the left side.

Locks should be on the left side, too.

#Comment by Brion VIBBER (talk | contribs)   22:07, 27 July 2011

Hmm... the lock icon in the image above looks to me like an "unlock to make changes" or maybe "this is an administrative action, beware!". Disabled buttons usually have the text grayed out, which is pretty standard across all OSs.

#Comment by Jorm (WMF) (talk | contribs)   22:08, 27 July 2011

Brion: I'm working with Kaldari right now about this exact thing.

The lock icon should only be appearing on "disabled" buttons, I think, in which case it would be greyed out.

#Comment by Kaldari (talk | contribs)   22:22, 27 July 2011

Don't get distracted by the lock icon, it was just an example :) Any icon can be put in either position by designating it as 'primary' or 'secondary'.

#Comment by Kaldari (talk | contribs)   07:51, 6 November 2011

Even though this went out in 1.17wmf, it missed the boat by 4 days to go into 1.18. Tags added (if it's not too late).

Status & tagging log