Legacy Stylesheets cleanup

Timo-Breumelhof commented 1 year ago

Is your feature request related to a problem?

The current Style sheets use quit a bit of legacy CSS and the "central" module.css files are way to specific which makes crating a custom theme much more complicated than necessary.

A few examples from "ActiveForums/Module.css" as an example:

.dnnPrimaryAction {
    box-shadow:none !important;
    -moz-user-select: none;
    box-sizing: border-box;
    display: inline-block;
    font-weight: 500;
    outline: 0px none;
    padding: 3px 7px 4px;
    text-align: center;
    text-decoration: none;
    text-shadow: none;
    cursor: pointer;
    border: 0px none;
    background: none repeat scroll 0% 0% #68AAC7;
    color: #FFF;
    line-height: 1.5;
}

IMO a module should reset the Themes button styling and if that' really needed, it should either use an extra class be more specific:

.activeForums .dnnPrimaryAction{
...
}

Next example:

input[type="text"], 
select, 
textarea, 
input[type="email"], 
input[type="search"], 
input[type="password"] {
    color: rgba(0, 0, 0, 0.44);
    font-size: 12px;
}

I don't see why the module needs to set the color of all input on the page..

Example from "ActiveForums/Themes/Module.css"

.afcontainer{font-size:12px; font-family:Tahoma, Arial, Verdana; padding:0px;text-align:left;}
* .afcontainer a,.afcontainer a:link, .afcontainer a:active,.afcontainer a:visited{color:#666;text-decoration:none !important;}
* .afcontainer a:hover{text-decoration:underline !important;}
.afcontainer TD {font-family:Tahoma;font-size:11px;}
.afgrid{font-size: 12px;color: #666;    font-family:  Tahoma, Arial, Verdana;   background-color: #fff;}

Although this does use more specific selectors, why would one want another Font in the Forum from the rest of the site? Also font sizes should be in "em" and scale with the font size set by the DNN Theme.

A lot of this CSS should be in the template style sheet IMO.

Describe the solution you'd like

Rename "ActiveForums/themes/_default/module.css" to "ActiveForums/themes/_default/template.css"
Clean up "ActiveForums/Module.css" and move the removed CSS to the legacy Template Folder the one above)
Split up the CSS in "ActiveForums/Themes/Module.css" in two. Move what's needed for all "ActiveForums/Module.css" the rest to the to the legacy Templates Style sheet "ActiveForums/themes/_default/template.css

Describe alternatives you've considered

None

Related to #93

johnhenley commented 1 year ago

With the changes for #93, I've created new folders: desktopmodules/activeforums/controlpanel/ and desktopmodules/activeforums/controlpanel/images/, and moving stuff out of themes that is specific to the control panel. Example is the jqueryui.css and its own images. They have to be copied into the theme and aren't even used by the theme. If you want to separate out the css that is specific to the control panel, that could go into the controlpanel folder. Just give me a filename.css.
Any remaining css that applies to any theme can go into the module.css in desktopmodules/activeforums
Theme-specific css goes into /portals/_default/activeforums_themes//theme.css

johnhenley commented 1 year ago

With the changes for #93, I've created new folders: desktopmodules/activeforums/controlpanel/ and desktopmodules/activeforums/controlpanel/images/, and moved stuff out of themes that is specific to the control panel. Example is the jqueryui.css and its owned images. They had to be copied into the theme and aren't even used by the theme. If you want to separate out the css that is specific to the control panel, that could go into the controlpanel folder. Just give me a filename.css. controlpanel.css?
My idea is that we focus on user-facing UI on this release
Any remaining css that applies to any theme can go into /desktopmodules/activeforums/module.css
Theme-specific css goes into /portals/_default/activeforums_themes//theme.css
At that point, theme-specific css should be minimal and really just colors, image specifics, and font gradations (definitely agree it should be em-based in relation to portal skin)
REALLY need to decide if templates belong to themes or are independent. If independent, there is slight risk that a template references a css class that's not in the theme. Probably not a big deal.

Timo-Breumelhof commented 1 year ago

@johnhenley sounds good. IMO a theme would be part of or a version of a template.a version of a template. I don't see a use case for separate, do you?

If you have a working version I can test with let me know and I'll spend time on the CSS cleanup..

johnhenley commented 1 year ago

Here's what I have in an upcoming PR:

any css that applies to any theme belongs in /desktopmodules/activeforums/module.css, which is delivered with each release, may be updated at any time, and probably should not be maintained by an administrator.

@Timo-Breumelhof, feel free to update that module.css to trim it down and remove unnecessary stuff as you've outlined in this issue, i.e. using more specific selectors.

Theme-agnostic css goes into /portals/_default/activeforums_themes/themes.css

My thoughts are that this gets delivered with initial install but not overwritten during upgrades. It would include stuff like @media query specific for avatar sizes as an example. Thoughts? Still trying to decide if we should have this intermediate layer...

Theme-specific css goes into /portals/_default/activeforums_themes//theme.css. For the upcoming PR this is community from the theme @Timo-Breumelhof did for dnncommunity.org.

During upgrade in this PR existing themes are moved to new locations and css will be renamed.

Timo-Breumelhof commented 1 year ago

@Timo-Breumelhof, feel free to update that module.css to trim it down and remove unnecessary stuff as you've outlined in this issue, i.e. using more specific selectors.

I will :-)

Theme-agnostic css goes into /portals/_default/activeforums_themes/themes.css

My thoughts are that this gets delivered with initial install but not overwritten during upgrades. It would include stuff like @media query specific for avatar sizes as an example. Thoughts? Still trying to decide if we should have this intermediate layer...

Theme-specific css goes into /portals/_default/activeforums_themes//theme.css. For the upcoming PR this is community from the theme @Timo-Breumelhof did for dnncommunity.org.

These two paths are the same. I'm not sure what the use case is for having a central theme.css? Shouldn't that be in a folder with the Theme name? (although as I said I personally would merge themes into the template folder)

During upgrade in this PR existing themes are moved to new locations and css will be renamed. Nice :-)

johnhenley commented 1 year ago

Let me know if you like this approach:

css like @media queries for avatar sizing that is used by all themes goes into a “parent” css, and then color-specific css for each theme goes into “child” css.

The forums module loads css in order from:

-- Applies to all instances of forums module across all portals and delivered with module – don’t update this as an admin, only via a PR. ~/desktopmodules/activeforums/module.css

NON-THEME-SPECIFIC css (applies to all instances; delivered with module updates and may be overwritten): ~/desktopmodules/activeforums/themes/theme.css

Theme-SPECIFIC css (assuming “community” is one theme name and flowers is second theme name; (applies to theme which is set for each module instance; base theme.css file is delivered by theme designer; custom/theme.css can be used by admin to override that. ~/desktopmodules/activeforums/themes/community/theme.css ~/desktopmodules/activeforums/themes/community/custom/theme.css ~/desktopmodules/activeforums/themes/flowers/theme.css ~/desktopmodules/activeforums/themes/flowers/custom/theme.css

WillStrohl commented 1 year ago

I'll default to Timo on these kinds of decisions. :)

Timo-Breumelhof commented 1 year ago

@johnhenley First of all thank you for all your work. Here are my thoughts on the subject: While I do like that approach there are a few things we might consider.A. I don't think "the default theme will never be overwritten" is the best approach TBH. That would mean we cannot make any fixes or changes and I'm sure we will find some after the first release. IMO DNN admin that wants to create a custom theme should copy the theme folder and make the changes in there, or if it's just minor changes, add an extra Stylesheet to overrule colors etc.

In that sense having a central themes stylesheet could be a good idea although I wonder if that could also be in module.css. I think we will know when we start doing the theme (if in the end it is just 3 lines of css for instance...)

So just to be sure A user would get the option to create a custom theme in /portals/_default/activeforums_themes/mytheme/theme.css right?

johnhenley commented 1 year ago

@johnhenley First of all thank you for all your work. Here are my thoughts on the subject: While I do like that approach there are a few things we might consider.A. I don't think "the default theme will never be overwritten" is the best approach TBH. That would mean we cannot make any fixes or changes and I'm sure we will find some after the first release. IMO DNN admin that wants to create a custom theme should copy the theme folder and make the changes in there, or if it's just minor changes, add an extra Stylesheet to overrule colors etc.

I was just concerned that an admin might modify the default/delivered theme and then a later upgrade would overwrite it without much of a warning. But if you think admins will copy the delivered theme and make their own, then I'm fine with that.

In that sense having a central themes stylesheet could be a good idea although I wonder if that could also be in module.css. I think we will know when we start doing the theme (if in the end it is just 3 lines of css for instance...)

Having an admin add only 3 lines of css to change a color is exactly my intention. It really only applies if you have multiple portals and forums instances, and therefore multiple themes. In that case I think most of the CSS can be in the generic themes.css. Or as you say, it could go in module.css, but I think that should be reserved for our use, not for an admin. Then if there are theme-specific colors or fonts, those go into each theme-specific theme.css. We will no doubt do some testing and figure it out eventually :)

So just to be sure A user would get the option to create a custom theme in /portals/_default/activeforums_themes/mytheme/theme.css right? Absolutely.

Timo-Breumelhof commented 1 year ago

@johnhenley I think we could add comments to the default theme, to warn that it will be overwritten on upgrade? for the test agreed :-)

Timo-Breumelhof commented 1 year ago

Fixed with #446

DNNCommunity / Dnn.CommunityForums