codyhouse / codyhouse-framework

A lightweight front-end framework for building accessible, bespoke interfaces.
https://codyhouse.co/
MIT License
1.16k stars 173 forks source link

Please go back to using partials #54

Closed peiche closed 4 years ago

peiche commented 4 years ago

I would like to go back to the stylesheet being built using Sass partials instead of everything existing within _base.scss or, at the very least, put mixins in a partial of their own. My stylesheet uses the new Sass module system, so one of my partials would use @use instead of @import. For example:

@use '~codyhouse-framework/main/assets/css/base/breakpoints';

.some-class {
    @include breakpoints.breakpoint(md) {
        //
    }
}

With the change introduced in v2.7, these partials no longer exist, forcing me to use @import globally again, which will eventually be removed altogether, per the Sass blog post.

seansean commented 4 years ago

Agreed, though for different reasons. I include only parts of the framework within my projects. The partials make the framework more flexible allowing users to implement the entire project or to pick and choose.

Love the framework -- thanks for all of your hard work!

sebastiano-guerriero commented 4 years ago

Hi there!

@peiche when you import the _base.scss file in your general _style.scss file, it includes also the breakpoints. Can you please give me an example of when it's convenient to import just the mixins? I'm trying to understand this use case.

@seansean can you please give me an example of which base partials would you import separately? I'm asking because most of the partials were intertwined (for example, you couldn't import the utility classes without importing the breakpoints and the spacing).

peiche commented 4 years ago

@sebastiano-guerriero that's how @import works, but not @use. Per the blog post:

@use only makes names available in the current stylesheet, as opposed to globally.

So while I do import the entire _base.scss in one partial, I now need to re-import the entire thing if I want to use the breakpoints mixin, for example, in a separate partial. Either that, or I need to switch back to using @import instead of @use, which I prefer not to do because the support for it will eventually be removed altogether.

seansean commented 4 years ago

@sebastiano-guerriero I use my own color and typography system so I exclude CodyHouse's counterpart. Here's a recent project base stylesheet:

/** Tools */
@import 'tools/all';

@import 'tokens/all';

/** CodyHouse Framework */
@import 'codyhouse-framework/main/assets/css/base/reset';
@import 'codyhouse-framework/main/assets/css/base/mixins';
@import 'codyhouse-framework/main/assets/css/base/breakpoints';
@import 'codyhouse-framework/main/assets/css/base/spacing';
@import 'codyhouse-framework/main/assets/css/base/grid-layout';
@import 'codyhouse-framework/main/assets/css/base/forms';

@import 'tokens/breakpoints';
@import 'tokens/spacing';

/** CodyHouse Overrides/extends */
@import 'custom-style/all';

/** Generic */
@import 'generic/all';

/** Elements */
@import 'elements/all';

/** Vendor */
@import 'vendor/all';

/** Objects */
@import 'objects/all';

/** Components */
@import 'components/all';

/** Modules */
@import 'modules/all';

/** CodyHouse utilities */
@import 'codyhouse-framework/main/assets/css/base/visibility';
@import 'codyhouse-framework/main/assets/css/base/accessibility';
@import 'codyhouse-framework/main/assets/css/base/util';

/** Utilities */
@import 'utilities/all';
sebastiano-guerriero commented 4 years ago

@peiche wouldn't it be more convenient to use:

@use '~codyhouse-framework/main/assets/css/base' as *;

.some-class {
    @include breakpoint(md) {
        //
    }
}

with no need to import single partials (e.g., in case you need the breakpoints + mixins + some-other-partial)? The @use shouldn't import the code multiple times even if you use it in multiple partials.

sebastiano-guerriero commented 4 years ago

@seansean we're working on a solution that would allow you to specify the base partials to include:

$cody-base-partials: 'accessibility', 'breakpoints', 'colors', 'forms', 'grid-layout', 'icons', 'mixins', 'reset','spacing', 'visibility', 'z-index';
@import 'codyhouse-framework/main/assets/css/base';

@import 'custom-style';
@import 'some-other-style';

$cody-base-partials: 'util';
@import 'codyhouse-framework/main/assets/css/base';

What do you think?

seansean commented 4 years ago

@sebastiano-guerriero That should work for my use case. Thank you for considering my request. I appreciate your team and your framework!

peiche commented 4 years ago

This change is the worst. It doesn't "simplify" anything. I really don't want to give up using this framework, especially since I prefer it to anything else out there. My only other option is to revert and never upgrade past v2.6.2. This proposed change would just be a "fix" that would be unnecessary if you didn't break anything in the first place.

sebastiano-guerriero commented 4 years ago

I understand your point. This change may not simplify your workflow and, like any other change, may not be welcomed by everyone. When we say "simplify", we refer to those users who got in touch because the difference between "base" and "custom-style" folders (with sort of duplicate file names) wasn't clear. It makes me think that many potential new users could be discouraged from using CodyFrame if we don't simplify this aspect.

At this point, we're monitoring the reactions to the change. We have had a couple of complaints so far. It appears the new SASS modules shouldn't be an issue. If you feel the new version is creating a problem in your workflow, let us know, and we'll def look into it.

peiche commented 4 years ago

No, importing the entire library with @use is actually an issue. My use case is in building a custom stylesheet which first imports the entire framework... no issue there. But then inside a specific class selector, I import some local partials which also now are importing the framework, solely for the use of mixins, like breakpoint, reset, etc. This ends up importing the entire framework again, inside this class. Not only does this complicate some overrides, it also massively bloats the size of the generated CSS file. I would still ask that you separate functions and mixins into separate partials.

sebastiano-guerriero commented 4 years ago

Can you please send me the source code (hello@codyhouse.co)? I'm not 100% clear on how you're importing local partials inside a class selector. From what I understood on the SASS documentation about modules, @use and @forward, unlike @import, shouldn't compile the same code multiple times, but just once (unless you're compiling in different .css files).

peiche commented 4 years ago

I decided to just copy the mixins I need to import from the framework into my own partials. This issue can be closed.

sebastiano-guerriero commented 4 years ago

OK! If you're using version 2.7.1, you can import (only) the mixins by writing:

$cody-base-partials: 'mixins';
@import 'codyhouse-framework/main/assets/css/base';