SimpleMachines / SMF

Simple Machines Forum — SMF in short — is free and open-source community forum software, delivering professional grade features in a package that allows you to set up your own online community within minutes!
https://www.simplemachines.org/
Other
595 stars 255 forks source link

Confusing CSS #5879

Closed pouvik closed 4 years ago

pouvik commented 4 years ago

So I started experimenting with theme design on 2.1. And one thing that SMF is always saying is it wants people to be encouraged to design themes, Most if not all the 2.1 themes already on Theme Site are pretty much made by people that have made multiple designs, but for a new person their is alot of confusing areas specially in the CSS, if they want to do any thing really different, alot of classes are lumped together, multiple reasons we all know no pount having 100 classes saying the same, argument it will load faster (im skeptical on that).

One simple example is this on:

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

inner_section id is quite an important one, for those that do not know its the section where you find the Time, News, Linktree ad Navigation. For me it took a fair few hours to find it, granted I have not done a theme in ages and not looked at the 2.1 code for themes in alot longer. For someone starting out this will bring more support topics of just prevent any unique designs being made from potentially new themers, also it does not work with the flow of placement, this is on line 3802 the rest of the section for this id is on line 1160.

Suggestion is remove #inner_section from this area so it looks more like

/* Those classes are sharing exact same gradient. */
/* Background of buttons */
.dropmenu li ul, .top_menu, .dropmenu li li:hover, .button,
.dropmenu li li:hover > a, .dropmenu li li a:focus, .dropmenu li li a:hover,
#top_section, #search_form .button, .quickbuttons li,
.quickbuttons li ul, .quickbuttons li ul li:hover, .quickbuttons ul li a:focus,
.popup_window{
    background: #fff; /* fallback for some browsers */
    background-image: linear-gradient(to bottom, #e2e9f3 0%, #fff 70%);
}

and line 1160 where we have the id already:

#inner_section {
    padding: 12px 10px 2px 10px;
    border-radius: 6px 6px 0 0;
}

changing it too

#inner_section {
    padding: 12px 10px 2px 10px;
    border-radius: 6px 6px 0 0;
    background: #fff; /* fallback for some browsers */
    background-image: linear-gradient(to bottom, #e2e9f3 0%, #fff 70%);
}
DiegoAndresCortes commented 4 years ago

I think @SychO9 is or will be working on that at some point

5665

SychO9 commented 4 years ago

My opinion on this is that I am not in favour of this change, I do not understand how it is better to turn this one block of code into multiple ones, if someone cannot find the class using the browser developer tools now how will they be able to find it if we tear it down to many blocks of code ? I don't see how that is related.

I'd love to hear @XinYenFon and @DiegoAndresCortes 's opinions on this though

I think @SychO9 is or will be working on that at some point

5665

No that is something different :-/

DiegoAndresCortes commented 4 years ago

Re-reading this I don't find it confusing at all and I think it's fine as multiple elements share the same attributes like the background color

DiegoAndresCortes commented 4 years ago

Additionally I agree that for a designer could be a bit of a headache separating these elements but some share certain areas that favor using the same colors.

XinYenFon commented 4 years ago

This is not an issue at all.

DiegoAndresCortes commented 4 years ago

Nope, more of a suggestion. I suggest we close this one under the argument provided by @SychO9