Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gulp, and select style #2

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

gulp, and select style #2

wants to merge 6 commits into from

Conversation

trusktr
Copy link
Member

@trusktr trusktr commented Apr 15, 2021

No description provided.

@trusktr trusktr changed the title Proposal card 57597321 gulp, and select style Apr 15, 2021
select[disabled] ~ label,
textarea[disabled] ~ label {
color: var(--muted-text);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works for

<input .../>
<label>...<label>

We will also need

<label>...<label>
<input .../>

and

<label>...<input .../><label>
<label><input .../>...<label>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was just a test. Thanks for the heads up

input[type='file' i],
summary {
cursor: pointer;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is still better to try to use multiple files, if possible. Wdyt? Is there a reason to put it in here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea is whatever is shared, have it in the same file. If you want to change the cursor it easier than go through different files.

input[type='reset' i],
input[type='file' i] {
cursor: pointer;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see this is (mostly) duplicate of the stuff in form.shared-is-test.css

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to have the duplicates?

margin: 0; /* 2 */
}

/* Change the inconsistent appearance in IE (opinionated) */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely let's make it all consistent!

button,
summary {
cursor: pointer;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another duplicate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this one, thanks

@trusktr
Copy link
Member Author

trusktr commented Apr 15, 2021

It's such a problem organizing code in plain CSS!

Random idea: What if we define certain entry points like index.css (the whole library) and form.css etc with particular areas. Then maybe we can make full build, section builds, and individual component builds.

-In the full build, we can use postcss to eliminate duplicate imports.

  • In the section builds (f.e. forms), we eliminate duplicate imports for each section, but we also re-name the imported variables for each section, in case the user will include multiple section builds.
  • components: similar to sections (like input, button, etc) but more granular (same build idea).

The downside is, if the user imports only parts, then they need to use different variable names for each part to ensure no collisions (f.e. forms.css might have --forms-muted-text and containers may have --containers-muted-text, even though they have the same values). If the user imports full.css (or index.css) build, then they only use --muted-text for all parts.

Get what I mean? A bit bad though. Still thinking....


About the gulp stuff, what does it do? I see it tries to operate on select.shared.css which doesn't exist.

@a133xz
Copy link
Contributor

a133xz commented Apr 21, 2021

@trusktr please check my last commits because I think it will answer my strategy for the variables and what Gulp does. Apologies for it because I didn't push the updates 😪

A recap:

I've created a Gulpfile to extract the styles from shared files that are specific to the element. You can try and experiment to see how it works. I did a regex that works well with pseudo-classes so it should be ok but not sure if is bullet-proof.

My idea is for each ____ELEMENT___ we'll generate a new file like this:

@import './shared/variables.css';
@import './shared/colors.css';

@import './utils/utils.css';

@import './elements/____ELEMENT___.css';

As you can see it's the same as src/index.css but replacing @import './elements/index.css' for the ____ELEMENT___

Gulp

Use npm run gulp and you'll see a new file compiled in dist called form.shared-is-test.css which has all the styles extracted from elements/form/form.shared-is-test.css for select. Ideally, this file will be called select.shared.css or similar.

In this proof of concept, the gulpfile.js is only working to extract the select classes from the elements/form/form.shared-is-test.css. You can play with the file/task.

Next

  1. Create the script to extract the styles for each element, not only the select as is currently the example
  2. Create a new final file for each element that will looks like this:
@import './shared/variables.css';
@import './shared/colors.css';

@import './utils/utils.css';

@import './shared/____ELEMENT___.shared.css';
@import './elements/____ELEMENT___.css';

Benefits

  • New file for EACH element with their colors, utils, etc, everything separated in different files, which means, if someone need to add another element you'll not have duplicated code
  • Better dev environment where we can have a better structure i.e create shared files (or just look at index how clean it is)
  • Posibility of creating critical styles where you have concatenated the utilities, variables, colors to save requests

Note: the select element is not styled yet with the colours because I'd like to work on the variables first

@trusktr
Copy link
Member Author

trusktr commented May 2, 2021

Next

  1. Create the script to extract the styles for each element, not only the select as is currently the example
  2. Create a new final file for each element that will looks like this:
@import './shared/variables.css';
@import './shared/colors.css';

@import './utils/utils.css';

@import './shared/____ELEMENT___.shared.css';
@import './elements/____ELEMENT___.css';

I see what you're saying. Wanna give it a try? I'm not sure I entirely see the full benefit yet. I mean, I think I see how the output may be, but also curious as to how it affects the dev experience.

One thing is, if the user writes

@import '/node_modules/@lume/basicss/dist/elements/select.css';
@import '/node_modules/@lume/basicss/dist/elements/input.css';

then this will still result in all stuff from variables.css, colors.css, and utils.css being duplicated. But I guess there's just no way around it!

I wonder if there maybe we can make a @layer (@import layer) plugin for poscss (or if one exists), and we can put shared code in a higher-priority layer.

@a133xz
Copy link
Contributor

a133xz commented May 6, 2021

@trusktr I guess since we're compiling with Gulp or PostCSS, we can create a file called:

  • select.only.css: with my proposal
  • select.css: with only the styles for the select
  • critical.css: with the variables, colors etc

Either way, I think it would be fun for me to try.

@trusktr trusktr changed the base branch from master to main September 7, 2022 16:04
@trusktr
Copy link
Member Author

trusktr commented Sep 7, 2022

deleted master, changed PR base to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants