liferay / liferay-js-themes-toolkit

MIT License
26 stars 28 forks source link

New 7.2 themes seem to confuse unstyled/styled base themes #395

Closed jbalsas closed 3 years ago

jbalsas commented 5 years ago

Based on https://liferay-community.slack.com/archives/C5H30KZ1A/p1570033569288400 and https://liferay-community.slack.com/archives/C67VDLMMF/p1570033390001700

Is anyone aware that for 7.2 themes > node_modules for styled theme contains less stuff than unstyled theme? They seem to be swapped around with unstyled surely meaning styled? Also no matter how hard I try even if I select styled I'm getting an unstyled theme. If I choose unstyled, I'm still getting unstyled?? Unstyled size on disk 5.67 MB Styled size on disk 444 KB This version of the liferay-js-themes-toolkit (9.4.0) npm i -g generator-liferay-theme@^9.0.0

Thanks. I did get a color change in _custom.scss applying so the theme is building correctly. Just choosing _unstyled as the base no matter what. If I choose unstyled on purpose the size of my theme war increases dramatically and I get all the images etc.

/cc @duracell80

duracell80 commented 5 years ago

Would it also be because of missing imports in styled?

This seems to go nowhere (line 7 /_imports.scss) @import "clay/base-variables";

There is _variables.scss but not _base-variables.scss

mwilliams2014 commented 5 years ago

Your screenshot (https://liferay-community.slack.com/files/U72NP3HAN/FNYRVQ16Y/custom_scss.png) looks like the Styled theme to me. If you want it to resemble the Classic theme (which I think is the styled look you imagine), you have to extend the Atlas Base theme, I believe. See https://portal.liferay.dev/docs/7-2/frameworks/-/knowledge_base/f/customizing-atlas-and-clay-base-themes Note that this adds default styling that may conflict with your custom styles @jbalsas can you confirm? I wasn't able to reproduce the file size swapping

john-co commented 5 years ago

I could not reproduce the issue described. The file sizes for styled theme is greater than the unstyled theme and the applied themes look as expected to me.

Styled

Unstyled

duracell80 commented 5 years ago

Can you look at the node modules folder and see that the _unstyled directory contains images and the styled does not. The styled directory is kilobytes big and the unstyled is megabytes big?? Unless _styled imports _unstyled in which case there's no choice? Just an illusion of choice? There's no way to generate a theme that doesn't have those images?

I think here in the documentation ...

"By default, Clay base variables are imported into the theme". From what I could see clay wasn't being imported into styled. Even the alerts looked messed up. The login box wasn't styled. How is that normal?

The hello world portlet isn't styled in the reproduction? In 7.0 the theme generator would produce a theme that was fully baked with classic. That's what I'm looking for the theme generator to do.

A theme where there are unstyled parts of the page when you've selected styled is a very confusing situation. Can there be an _ultrastyled choice??

Ultrastyled would then import atlas via the theme generator. Choosing styled theme seems to produce an incomplete theme.

https://portal.liferay.dev/docs/7-2/frameworks/-/knowledge_base/f/customizing-atlas-and-clay-base-themes

duracell80 commented 5 years ago

I guess the user story here is that as a theme developer I want the theme generator to produce a mirror copy of classic theme so that I have a fully baked base theme to customize with variables.

So Atlas isn't included by default when generating a theme? I need the hello world portlet to be styled by default when using the _styled theme.

jbalsas commented 5 years ago

Hey @duracell80!

Yeah, I think there's a bit of confusion coming from the docs and in terms of nomenclature and expectations.

The https://portal.liferay.dev/docs/7-2/frameworks/-/knowledge_base/f/clay-css-and-themes tutorial might provide some additional rationale into what you're seeing:

Clay CSS is bundled with two sub-themes: Clay Base and Atlas. Clay Base is Liferay Portal’s Bootstrap API extension. It adds all the features and components you need and inherits Bootstrap’s styles. As a result, Clay Base is fully compatible with third party themes that leverage Bootstrap’s Sass variable API.

Atlas is Liferay Portal’s custom Bootstrap theme that is used in the Classic Theme. Its purpose is to overwrite and manipulate Bootstrap and Clay Base to create its classic look and feel. Atlas is equivalent to installing a Bootstrap third party theme.

Other than a glitch that caused a publication of the classic theme in 7.0 and in some of our tooling, we've never really supported extending from the classic theme. As it is, the classic theme is a final implementation of a theme that we need control over. Making it a dynamic extension point makes it very difficult for us to evolve.

Thus, we provide:

Keep in mind that styled extends unstyled in a way. The tooling always overlays those too when you extend styled. That's why you only see the images in unstyled, but they should end in your styled-based themes too.

I think at this point, starting from classic has been a common enough request that we need to consider something different than the extension mechanism (which would bound us to keep compatibility). Maybe we can add something like a "kickstart" command with the generator that would dump a copy of classic into your theme... 🤔

/cc @david-truong, @marcoscv-work?

duracell80 commented 5 years ago

Sorry for taking a while to get back to this.

There is another person having the same issue https://liferay.dev/forums/-/message_boards/message/114183284

It does make sense with the images being in unstyled, therefore it would be bigger, I had forgotten about that caveat. The issue though I think is that when we ran the generator 4 years ago we got a theme that was styled ... now we don't and it's not clear why that changed or when atlas stopped being included in a generated styled theme.

I still like _ultrastyled, sounds so cool!

wincent commented 3 years ago

We've just migrated this project to the liferay-frontend-projects monorepo in the name of consolidation. As this issue is quite old now, I'm not going to transfer it automatically. If you consider it to be still relevant, please create a fresh issue over in the new repo.