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

Reduce build size by optimizing redundant css #275

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Reduce build size by optimizing redundant css #275

wants to merge 3 commits into from

Conversation

Raiondesu
Copy link

Description

This change removes unnecessary rules by encapsulating the functionality in a single selector, making sources easier to maintain while also reducing build size.

Check List

instruction : terminal command

  • run the build script grunt
  • open index.html in a browser & resize to test visual issues

.col-xs-offset-10,
.col-xs-offset-11,
.col-xs-offset-12 {
[class|="col"] {

Choose a reason for hiding this comment

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

That is not gonna work.
|= means class attribute starts with col.
What if a developer will add additional class in the beginning of the attribute value? Like <div class="something col-sm-..."?

I would use something like [class*="col-xs]..., [class*="col-sm]....
But again, even that approach restricted a developer on adding some utils classes like col-xs-whatever.

Copy link
Author

@Raiondesu Raiondesu Sep 26, 2018

Choose a reason for hiding this comment

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

About first one - good point. Didn't think about it for some reason. Gonna fix now.

But again, even that approach restricted a developer on adding some utils classes

Well, it doesn't, actually.
[class*="col-"] would clearly cut it for both usecases, since it would match both the usual col classes and custom utility ones too:

<element class="col-xs-6 col-xs-make-it-very-cool col-lg-make-it-very-cool">

In any case, current waterfall of classes every 100 or so lines does not cut it for the second use-case either, so the only drawback to defining common rules as [class*="col-"] would be that all similarly-looking classes would fit this selector (which I personally view more like a nice feature).

Note: build size still smaller by a chunk.
@@ -224,6 +224,13 @@
width: var(--container-sm, 46rem);
}

[class*="col-sm"] {

Choose a reason for hiding this comment

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

speaking of reducing bundle size, why you cannot to put styles in one place and separate attributes with ,?

[class*="col-xs"],
[class*="col-sm"],
[class*="col-md"],
[class*="col-lg"] {
    box-sizing: border-box;
    flex: 0 0 auto;
    padding-right: var(--half-gutter-width, 0.5rem);
    padding-left: var(--half-gutter-width, 0.5rem);
}

Copy link
Author

Choose a reason for hiding this comment

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

This depends on expected behavior.
The way you mentioned, common styles would apply regardless of screen size, whereas in this PR styles still apply in regards to current screen size and xs/sm/md/lg modifiers as it originally was.

Copy link
Author

@Raiondesu Raiondesu Sep 26, 2018

Choose a reason for hiding this comment

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

I don't know which one is preferred though, so I view both versions as valid.

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