conversionxl / aybolit

Lightweight web components library built with LitElement.
https://conversionxl.github.io/aybolit/
MIT License
7 stars 8 forks source link

feat(cxl-ui): add login styles #243

Closed saas786 closed 1 year ago

saas786 commented 1 year ago

https://app.clickup.com/t/3ccnz6j

github-actions[bot] commented 1 year ago

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js 65.72 KB (+0.04% 🔺)
kylebrodeur commented 1 year ago

Task linked: CU-3ccnz6j Change lost my password link style

lkraav commented 1 year ago

Is writing a SASS compile directives

What are "SASS compile directives"?

pawelkmpt commented 1 year ago

What are "SASS compile directives"?

mix.sass(`${scssPath}/cxl-login.scss`, `${distPath}/styles/cxl-login.css`);
saas786 commented 1 year ago

Replying to both institute-theme & aybolit PRs.

Originally created: https://github.com/conversionxl/aybolit/commit/110f6bd#diff-31fbf03c262224ebb188944a78ac8e2ac946dbbd6dfa52ef249d3772b74142f4

It pulls lumo colors & we can use our cxl brand color for links. But saw existing login page using blue links, so reverted it.

chrome_n0KOvymsHy

put all in Custom Login plugin admin screen cxl.com/institute/wp-admin/options-general.php?page=custom-login (all in one place for future refactor)

Using custom css inline is not ideal, as we can't utilize the lumo styles now or later (on login page).

(preferred) or, get it all out of Custom Login, and put into a wp-theme-lib SCSS file, then disable plugin (major advantage: WPS will also gain a reference login experience)

Hmmmm, should create a new Login component in wp-theme-lib like CXLUi/WPLogin.php?

ref: https://github.com/conversionxl/wp-theme-lib/tree/master/src/CXLUi

Sure, can move custom css from options-general.php?page=custom-login to current PR.

Can you check what would be the size of a bundle that includes only cxl-lumo-styles/color.js? Ideally we would be able to build a minimal bundle of at least color and typography for specialty screens like login.

I have given it a thought, and I don't see us using any JS on WP Login page, so using a bundle is overkill, as my estimate was it will be at least 20KB+, where if we just serve css without cxl-ui bundle, its hardly gonna be 1KB+ or similar. Even if we use: https://github.com/conversionxl/aybolit/commit/110f6bd#diff-31fbf03c262224ebb188944a78ac8e2ac946dbbd6dfa52ef249d3772b74142f4 (lumo colors).

@pawelkmpt I would not accept any PRs without an accompanying story. Otherwise, validating whether any such shipped CSS makes any sense becomes super complicated, because you need make roundtrips to pulling Aybolit changes in wp-theme-lib.

Story only make sense, if you want to see https://www.figma.com/file/T7CzLFP2K5yHPShh1d7tlK/Login-Flow-Redesign?node-id=0%3A1&t=RYBfeGkrBn69I2HD-1 in action, and it would require some time & effort to create story for each login screen view, it's an overkill for our custom login css.

As far as I remember (can look for such threads), we wanted to move away from wp-theme-lib or themes scss (unless an override is needed in specific theme), and instead server styles via aybolit, GravityForm css was major step towards this?

I'm not clear on why it's necessary to add new cxl-ui devDeps. Also not sure whether this new CSS build strategy makes sense as-is, or if it could be better 🤔 it would help if there was another CSS implementation example to use this system - what is an important non-web-component thing we need to style - Woo Checkout comes to mind immediately.

There is no new css build strategy, it was always there, we just had no need to server simple css via aybolit, now we have, so utilizing it. There is component specific css & there is none component css, and both have its own use and purpose.


Let me know your what you think.

pawelkmpt commented 1 year ago

Using custom css inline is not ideal, as we can't utilize the lumo styles now or later (on login page).

I agree. But current goal is to improve login experience with low cost before Christmas. We're not looking for "ideal for developer" now. We're looking for "better for user".

lkraav commented 1 year ago

I have given it a thought, and I don't see us using any JS on WP Login page, so using a bundle is overkill, as my estimate was it will be at least 20KB+, where if we just serve css without cxl-ui bundle, its hardly gonna be 1KB+ or similar. Even if we use: https://github.com/conversionxl/aybolit/commit/110f6bd#diff-31fbf03c262224ebb188944a78ac8e2ac946dbbd6dfa52ef249d3772b74142f4 (lumo colors).

Noticed upstream https://github.com/vaadin/web-components/pull/4935

Even though login is a stupidly "special" screen, maybe one of its kind in the whole app - maybe spending that 20kB on the micro-bundle makes more sense than trying to build CSS bundles..?

Story only make sense, if you want to see https://www.figma.com/file/T7CzLFP2K5yHPShh1d7tlK/Login-Flow-Redesign?node-id=0%3A1&t=RYBfeGkrBn69I2HD-1 in action, and it would require some time & effort to create story for each login screen view, it's an overkill for our custom login css.

Story makes perfect sense if it only has the main login box, with no additional steps storied.

It's already much ahead of "nothing" by that point.

pawelkmpt commented 1 year ago

We don't need code solution now. Closing.

saas786 commented 1 year ago

Even though login is a stupidly "special" screen, maybe one of its kind in the whole app - maybe spending that 20kB on the micro-bundle makes more sense than trying to build CSS bundles..?

@lkraav

It resulted in about 240kb+.

explorer_bQXNAMpS3j

lkraav commented 1 year ago

It resulted in about 240kb+.

Quite disappointing, indeed. Even though I'm not sure what gets pulled into vendor-login.js, extracting CSS for cross-usage seems like the only viable option.

saas786 commented 1 year ago

It resulted in about 240kb+.

Quite disappointing, indeed. Even though I'm not sure what gets pulled into vendor-login.js, extracting CSS for cross-usage seems like the only viable option.

Just importing colors,typography packages now, and still seem to work, and around 32kb+ size.

explorer_YmZSqCKvW4

lkraav commented 1 year ago

Just importing colors,typography packages now, and still seem to work, and around 32kb+ size.

That was the original idea. What did you import into the 240 kB package?

For 32 kB total, what does bundle analyzer show getting pulled in?

saas786 commented 1 year ago

That was the original idea. What did you import into the 240 kB package?

I wasn't sure if just importing colors,typography will be enough, just went with full cxl-lumo-styles import, which additionally had themes, icons, global styles etc.

For 32 kB total, what does bundle analyzer show getting pulled in?

firefox_YiopGAXG5A

lkraav commented 1 year ago

For 32 kB total, what does bundle analyzer show getting pulled in?

Yeah, vaadin-themable-mixin I also reached tracing by hand, and gave up a that point.

Unless we start needing other Lit-things, some CSS bundle build process is certainly more optimal.

Even though 80/20 + speed question arises: should we still load this small bundle, until we get the real thing done?

CSS bundle doesn't sound like 1-2 hr job :worried:

saas786 commented 1 year ago

CSS bundle doesn't sound like 1-2 hr job 😟

Its your preference,

we can use current JS bundle.

Or I can revert to custom css, which only includes css on login page.