News:

SMF 2.1.4 has been released! Take it for a spin! Read more.

Main Menu

2.1 - Easy CSS crud reduction, if anyone can be bothered.

Started by Antechinus, September 27, 2019, 09:30:53 PM

Previous topic - Next topic

Antechinus

#20
That "fix" will break functionality. It needs to be like this:

<a' . (!empty($li['href']) ? ' href="'.$li['href'].'"' : ' href="javascript:void(0);"') . (!empty($li['javascript']) ? $li['javascript'] : '') . '>

That will work. Yes, I have tested both. ;) Note that it does need the space in front of the href.

Antechinus

While messing with that, I've come up with some better CSS for the quickbuttons. The short version is that putting the styling on the li means no indication on li>a:focus, whereas if you put the styling on the anchor it looks consistent on hover and focus.

Also, it makes sense to put the .quickbuttons and .button code together in the file, because they share a lot of code. At the moment the code for several things is all over the place, which encourages people to lose the plot.

What I've done on local is remove the .button and .quickbuttons declarations that were jammed in with a pile of other things towards the end of the file (around Line 3700 ish), and instead just declare the necessary gradient for .button and .quickbuttons once, where it will be of some use to the average mug who wants to tweak their theme. So the stuff at the end of the file is now like this:

/* Those classes are sharing exact same gradient. */
/* Background of buttons */
.dropmenu li ul, .top_menu, .dropmenu li li:hover,
.dropmenu li li:hover > a, .dropmenu li li a:focus, .dropmenu li li a:hover,
#top_section,
.popup_window, #inner_section {
background: linear-gradient(#e2e9f3 0%, #fff 70%);
}


Which frankly is still a bit silly as it really should be up with the elements it is referring to. The file is severely bloated, but putting that particular declaration at the end of the file does nothing to reduce that. The big gains for brevity are elsewhere. Haven't got that far yet.

Anyway, the new (recommended, looks better and functions better) code for .button and .quickbuttons is:

* Styles for the standard button lists.
------------------------------------------------------- */
.buttonlist, .buttonrow, .pagelinks {
z-index: 100;
padding: 5px 0 5px 0;
margin: 5px 0 5px 0;
}
a.button, .quickbuttons>li>a {
display: inline-block;
padding: 0 8px;
background: linear-gradient(to bottom, #fff 0%, #e2e9f3 70%);
color: #444;
font-size: 0.85em;
line-height: 2em;
text-transform: uppercase;
text-decoration: none;
height: calc(2em + 2em * (0.9 - 0.85)); /* "input" font size minus ".button" font size */
border: 1px solid #ddd;
border-right: 1px solid #bbb;
border-bottom: 1px solid #aaa;
border-radius: 3px;
box-shadow: 1px 1px 1px #ddd;
}
.pagesection .button {
color: #346;
}
.button:hover, .button:focus,
.quickbuttons>li>a:hover, .quickbuttons>li>a:focus {
color: #222;
border: 1px solid #ddd;
border-left: 1px solid #bbb;
border-top: 1px solid #aaa;
box-shadow: -1px -1px 2px #ddd, -1px -2px 4px #ddd inset;
}
.button:hover, .button:focus {
color: #af6700;
}
/* the active one */
.button.active {
background: #557ea0;
color: #fff;
font-weight: bold;
border: 1px solid #558080;
text-shadow: 0 0 1px #000;
}
.button.active:hover, .button.active:focus {
color: #ffc187;
background: #557ea0;
box-shadow: none;
}
/* In a .buttonrow, the buttons are joined together */
.buttonrow {
margin: 0 5px;
}
.buttonrow .button {
display: table-cell;
border-radius: 0;
}
.buttonrow .button:first-child {
border-radius: 3px 0 0 3px;
}
.buttonrow .button:last-child {
border-radius:  0 3px 3px 0;
}
/* in a titlebg, the buttonlist is of small height */
.titlebg .buttonlist {
margin: 0;
padding: 0;
}
/* The quick buttons */
.quickbuttons {
margin: 11px 0 4px 0;
clear: right;
float: right;
text-align: right;
font-size: 0.75rem;
}
#recent .quickbuttons {
margin: 0;
}
.quickbuttons>li {
float: left;
}
.quickbuttons>li>a {
display: block;
height: auto;
padding: 0 4px;
color: #222;
line-height: 1.375rem;
border: 1px solid #ddd;
border-right: 1px solid #bbb;
border-bottom: 1px solid #aaa;
/* This is a case where shorthand hex
/* is best for a box-shadow. It always
/* overlays a blank background, so there
/* is no benefit in the longer rgba syntax. */
box-shadow: 1px 1px 1px #ddd;
border-radius: 0;
}
.quickbuttons>li:first-child>a {
border-radius: 4px 0 0 4px;
}
.quickbuttons>li:last-child>a {
border-radius: 0 4px 4px 0;
}
.quick_edit, .post_options {
position: relative;
}
/* Drop part of QuickButtons */
.post_options ul {
display: none;
position: absolute;
top: 100%;
right: -1px;
z-index: 90;
margin-top: 4px;
padding: 6px 0;
background: #fff;
font-weight: normal;
text-align: left;
border: solid 1px #999;
border-left: solid 1px #aaa;
border-top: solid 1px #bbb;
border-radius: 4px 0;
/* This is a case where rgba syntax is
/* worthwhile for a box-shadow, as it
/* overlays post text, which should
/* remain visible for best effect. */
box-shadow: 2px 3px 3px rgba(0, 0, 0, 0.2);
}
.post_options:hover ul {
display: block;
}
.post_options ul a {
display: block;
width: 12em;
padding: 0 6px;
line-height: 2.2em;
text-decoration: none;
border: 1px solid transparent;
border-width: 1px 0;
}
.post_options ul a:hover, .post_options ul a:focus {
background: #e2e9f3;
border-color: #c4cbd3;
}
.quickbuttons li.inline_mod_check {
padding: 1px;
}
/* Note: The next declarations are for keyboard access with js disabled. */
/* @WIP: Note: these have been broken by someone. Does lead dev want them fixed? */
.quickbuttons ul li a:focus {
margin: 0 -9910px 0 9910px;
}
/* Cancel for hover and/or js access. */
.quickbuttons ul li:hover a:focus, .quickbuttons ul li a:focus {
margin: 0;
}


Note the bit at the end. It is up to Sesq (or whoever is Big Dev Numero Uno these days) if he wants all drop menus being keyboard  accessible with js disabled. I know he made approving noises when Gwen was talking about a11y a while back.

If he doesn't want that, the redundant code can be removed. If he does want it, it is easily achieved providing (and this is important) people are prepared to take the code given and to not (this means not) frig around with anything they do not understand (which, let's face it, every code junkie loves doing :P ).

SychO

Quote from: Antechinus on September 29, 2019, 08:45:51 PM
That "fix" will break functionality. It needs to be like this:

<a' . (!empty($li['href']) ? ' href="'.$li['href'].'"' : ' href="javascript:void(0);"') . (!empty($li['javascript']) ? $li['javascript'] : '') . '>

That will work. Yes, I have tested both. ;) Note that it does need the space in front of the href.

looking at the image, that's not the exact edit that is posted :P
here's the exact edited line <a href="' . (!empty($li['href']) ? $li['href'] : 'javascript:void(0);') . '"' . (!empty($li['javascript']) ? $li['javascript'] : '') . '>

it's the same just declares href once instead.




I'll take a look at the other code changes you posted later(gotta run), but from reading what you are saying, totally sounds good :)
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

#23
Ah. Righty o then. I missed that you'd changed the front of it too. My bad.

And come to think of it, that quickbuttons CSS can probably be cleaned up a bit. Unless I'm missing something, I don't think that ul actually needs to be floated. There are probably a couple of other declarations that could be made more concise too.

SychO

Ah now I know why the style was applied to the list items instead of the anchors, you can turn on "Checkboxes" for quick moderation, if the li is styled then the checkbox will look consistent with the rest, otherwise you got yourself a checkbox next to the quicbuttons that is not styled like the rest, I don't think wrapping the input with an anchor is a good idea though.

I like that you got rid of the selectors that were using the "not()" function, that one can be a real pain because its priority level is high.
I made some edits though; I put back the "cursor: pointer;" for the sake of submit inputs, and "text-decoration: none;" on the button hovering effect (underline in a button is .. eh weird) and the gradient background for the ".post_options ul" element, I don't dislike the style, I just think it's inconsistent with the rest of the theme and should retain the same style. I dropped the "font-size: 0.75rem;" on the quickbuttons, it is unnecessary since the anchors have their font-size set, however they look smaller than before, too small.

Also the hovering effect on a buttonrow button which is sibling to an active button, the shadow effect is kinda weird.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

#25
In that case I'd suggest just having an extra declaration for the one that wraps the checkbox, if you want that look. It wouldn't take much (presumably just add .inline_mod_check in one place). That way at least the anchors all work consistently on hover or focus. I know Apple makes focus have a distinct outline, but Firefox and friends don't (and the Apple outline is seriously ugly anyway).

IIRC originally we didn't bother wrapping the checkboxes in any styling. They were just checkboxes sitting after the buttons, which I thought was fine, but wrapping it in an li is fine too if you prefer that. And I must have missed the text-decoration thing.

TBH I think some of the current styling details (which are not original) are pretty ugly, so personally I'd have no qualms about making them less ugly. For example, the gradient on buttons and quickbuttons had been inverted from the original, which made them look rough as guts, so I flipped it back to the original. Similar deal with the drops. I think they're pretty gruesome at the moment, so making them a bit less gruesome is not a bad thing (while keeping the theme as a whole the same). Especially if it can be done while saving some code. I'm not going to start World War 3 over it though. :)

QuoteAlso the hovering effect on a buttonrow button which is sibling to an active button, the shadow effect is kinda weird

Not sure what you mean there. Must have missed that too. Those are only used in the calendar and maybe a couple of other places, right? I'll take a look.

ETA: I see what you mean now. Easy tweak. I'll do something for it if you like.

ETA2: Aha. I see what the problem is with the checkboxes. There's no class assigned to the li, which is inconsistent with the rest of the buttons. I think it would be sensible to assign a class. They used to be .inline_mod_check, which is why there's a declaration for that in the file.

'quickmod' => array(
'class' => 'inline_mod_check',
'id' => 'in_topic_mod_check_'. $output['id'],
'custom' => 'style="display: none;"',
'content' => '',
'show' => !empty($options['display_quick_mod']) && $options['display_quick_mod'] == 1 && $output['can_remove'],
)


ETA3: Hey there is no button text underline on hover on my box. Not with the code I was running.

Antechinus

Oh hey: what's the minimum PHP version that 2.1 is comfortable with? I've been a bit lazy about upgrading my local, but for the moment I'd like to keep the ability to run some old 1.1.x stuff for reference, so if running on 5.6 isn't going to cause any problems I might leave it for now. If it's going to cause issues, I'll upgrade the local to 7.1 and figure out something else for running the old stuff.

shawnb61

Address the process rather than the outcome.  Then, the outcome becomes more likely.   - Fripp

Kindred

Слaва
Украинi

Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

"Loki is not evil, although he is certainly not a force for good. Loki is... complicated."

Antechinus

#29
Ok, so 5.6.26 should be fine for now. Handy to know.

Hold on to your hats, guys. I'm about to post code on Github. :P

ETA: Used GitHub without even swearing once. It's a miracle.

https://github.com/SimpleMachines/SMF2.1/pull/5812#issuecomment-536790818

Am not, that means not, going to fire up Sourcetree though. I'm quitting while I'm ahead. :D

shawnb61

Address the process rather than the outcome.  Then, the outcome becomes more likely.   - Fripp

Antechinus

Hey this next bit isn't a bugfix as such. It's just something I got it into my head to play with briefly. I thought I might as well put it up here to see what people think of it.

The new admin home page has always looked a bit messy to me. All the stuff is down the bottom, and although it is in rows it gives the visual impression of not really being structured. It's a bit hard to find things quickly as it doesn't seem to scan well. So I threw a few CSS tweaks at it, and I think it's a better look for the same content. I find it easy and natural to scan.

This is all using flex, with a parent div set to flex-wrap, auto width for the fieldsets on larger screens, and 48% width kicking in at 640px screen width. The last section (maintenance) just goes to full width automatically as it's the only one in its row. It should all be bulletproof, and it will have automatic RTL support without extra code being required.

The only markup change required is one wrapper div around the bunch of fieldsets, to give the required flexbox parent. The rest is just CSS tweaks.

SychO

Yea it seems like the 'inline_mod_check' class got lost somewhere along the way, anyway I'll restore that and use it like you did.

is there any reason for the marked redundant code ?

.quickbuttons > li > a, .inline_mod_check {
display: block; /* can be removed */
height: auto;
padding: 0 4px;
color: #222;
line-height: 1.375rem;
border: 1px solid #ddd; /* can be removed */
border-right: 1px solid #bbb; /* can be removed */
border-bottom: 1px solid #aaa; /* can be removed */
box-shadow: 1px 1px 1px #ddd; /* can be removed */
border-radius: 0;
}
.button, .quickbuttons > li > a, .inline_mod_check {
display: inline-block;
padding: 0 8px;
background: linear-gradient(to bottom, #fff 0%, #e2e9f3 70%);
color: #444;
font-size: 0.85em;
line-height: 2em;
text-transform: uppercase;
cursor: pointer;
height: calc(2em + 2em * (0.9 - 0.85));
border: 1px solid #ddd;
border-right: 1px solid #bbb;
border-bottom: 1px solid #aaa;
border-radius: 3px;
box-shadow: 1px 1px 1px #ddd;
}


I also removed the over-specification here
a.button, ... to .button, ...

buttons aren't always anchors.

Also the shadow effect you used on buttons is nice assuming buttons will always be used on light elements, when they're used in elements such as the cat_bar which has a blue like background, the shadow effect is bad, so I do prefer how the buttons looked before to be honest.

I haven't pushed any of the changes to the PR yet, will do so soon.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

SychO

wish we could make use of the var() function though, too bad it's not supported across all browsers.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Antechinus

Well, if you want a different shadow effect on dark backgrounds all you need to do is define it. I didn't change that though. I only reversed the direction of the background gradient. Frankly I thought the way they were before was ugly as hell on light backgrounds, which I why I put them back to the original gradient direction. That still works on a catbg background. It's only the perimeter styling which might not work.

And, having just taken a look at it, I agree the perimeter styling could be improved. TBH I'm not entirely sure if the perimeter styling is even best on light backgrounds. I think it could still be tweaked slightly. Will have a play with it if you like.


Quotebuttons aren't always anchors.

Yes it still seems fine without the tag there.

.quickbuttons > li > a, .inline_mod_check {
display: block; /* can be removed */
height: auto;
padding: 0 4px;
color: #222;
line-height: 1.375rem;
border: 1px solid #ddd; /* can be removed */
border-right: 1px solid #bbb; /* can be removed */
border-bottom: 1px solid #aaa; /* can be removed */
box-shadow: 1px 1px 1px #ddd; /* can be removed */
border-radius: 0;
}


The display:block is necessary there as they are defined as inline-block earlier in the file. That's because I wanted to pick up the existing styling from the .button class for the .quickbutton anchors, without having to declare it all over again. See back here:

.button, .quickbuttons>li>a, .inline_mod_check {
display: inline-block;
padding: 0 8px;
background: linear-gradient(to bottom, #fff 0%, #e2e9f3 70%);
color: #444;
font-size: 0.85em;
line-height: 2em;
text-transform: uppercase;
height: calc(2em + 2em * (0.9 - 0.85)); /* "input" font size minus ".button" font size */
border: 1px solid #ddd;
border-right: 1px solid #bbb;
border-bottom: 1px solid #aaa;
border-radius: 3px;
box-shadow: 1px 1px 1px #ddd;
}


It makes sense to re-use that code for quickbuttons, but it also makes sense to declare the .quickbuttons anchors as block later, because otherwise you have to mess around with the gaps between buttons that will be forced by inline-block.

It's ok dealing with that gap once on the li's, to avoid having to clear floats and to get automatic support for RTL, but dealing with it twice on the anchors as well as the li's is a bit pointless. It's simpler to just declare the anchors as block display so they fill the li's.

But yes, those borders and box-shadows can be removed if you like. I was thinking of tweaking them some more, so had left them in both places so I could try comparisons, but not needed in the repo as such. :)

Antechinus

#35
Hey just found a related bug. The quickbuttons in the strip on the profile>alerts page have a checkbox at the end, but that's not picking up any styling on the li. Not sure at the moment where that one is defined (haven't looked yet) but giving it the .inline_mod_check class seems like the sensible option.

ETA: Oh and would suggest changing the margin on the .quickbutton li's. My suggested code is this:


margin: 0 -.2rem 0 0;


But this works better (automatic for RTL, and slightly cleaner looks IMO):


.quickbuttons>li {
position: relative;
display: inline-block;
margin: 0 -2px;
}


Buttons also seem to need a text-decoration: none on hover. I'm sure I tested that, but I may have lost the plot somewhere, because underlining on hover is back. :P

So this works:

.button:hover, .button:focus,
.quickbuttons>li>a:hover, .quickbuttons>li>a:focus {
color: #222;
cursor: pointer;
text-decoration: none;


In which case it's not needed back here:

a.button, .quickbuttons>li>a, .inline_mod_check {
display: inline-block;
padding: 0 8px;
background: linear-gradient(to bottom, #fff 0%, #e2e9f3 70%);
color: #444;
font-size: 0.85em;
line-height: 2em;
text-transform: uppercase;
text-decoration: none; /* can be removed */


That one is not needed as they are defined that way (global for all anchors) at the start of the file. It's only the hover that is a problem.

Do you want me to do some more testing and get all of that into one sensible post? :D

ETA: Really, with the way the border styles are, it would make more sense to do the initial declaration as border: 1px solid; and then do the different colors by using the border-color attribute.

border: 1px solid #ddd;
border-right: 1px solid #bbb;
border-bottom: 1px solid #aaa;


Changed to:

border: 1px solid;
border-color: #ddd #bbb #aaa #ddd;


Is less verbose overall, since on hover and focus it's only the color that needs to be declared again:

border-color: #aaa #ddd #ddd #bbb;

Antechinus

#36
Hey the only place that seems to have the .button class on a dark background is the admin search bar. Are there any other cases that I'm missing?

If that's the only one, how about just moving it to the information panel beneath the cat_bar? I did that yesterday on my 2.0.x mutant and IMO it's a much better look. Currently the admin search bar on 2.1 is a mess on narrower screens anyway.

ETA: Or, since I know what it's like trying to convince people to do something different, if you're determined to leave the admin search inside the catbg, so you can have fun fighting to get a decent responsive layout from that arrangement :D, then just give it specific styling in admin.css.  That way, index.css is not cluttered by declarations that only admins will need, and you can style template_admin_quick_search() any way you like.

shawnb61

Excellent thread, Ant & SychO.   

Just a note this is being tracked in GitHub over here:
https://github.com/SimpleMachines/SMF2.1/pull/5812
Address the process rather than the outcome.  Then, the outcome becomes more likely.   - Fripp

Antechinus

Yes, I know. :D This thread is getting a bit messy. I'm going to try to rationalise all of this stuff on local soon.

SychO

Quote from: Antechinus on October 02, 2019, 05:41:58 PM
Hey just found a related bug. The quickbuttons in the strip on the profile>alerts page have a checkbox at the end, but that's not picking up any styling on the li. Not sure at the moment where that one is defined (haven't looked yet) but giving it the .inline_mod_check class seems like the sensible option.

No worries, I took care of all the quickmoderation checkboxes everywhere.

Quote from: Antechinus on October 02, 2019, 05:35:55 PM
The display:block is necessary there as they are defined as inline-block earlier in the file. That's because I wanted to pick up the existing styling from the .button class for the .quickbutton anchors, without having to declare it all over again. See back here:

I don't think it is, since the quickbuttons list items are floated left.

Quote from: Antechinus
Buttons also seem to need a text-decoration: none on hover. I'm sure I tested that, but I may have lost the plot somewhere, because underlining on hover is back. :P

I already fixed it like I mentioned previously.

Quote from: Antechinus
ETA: Really, with the way the border styles are, it would make more sense to do the initial declaration as border: 1px solid; and then do the different colors by using the border-color attribute.

Lovely :D

You might want to just pull the branch where I pushed all changes and take a look, pulling branches is a lot easier than manually applying changes, it helps in the review process.
Checkout My Themes:
-

Potato  •  Ackerman  •  SunRise  •  NightBreeze

Advertisement: