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

Consider putting sass variables and mixins in separate files from styles #9192

Closed
abierbaum opened this issue Nov 16, 2016 · 10 comments
Closed
Assignees

Comments

@abierbaum
Copy link

Ionic 2: Feature Suggestion

When using ionic in a project where we have our own build, it is difficult/impossible to make use of the variables and mixins declared in Ionic because the settings used for the styles are mixed in with the styles themselves.

Consider for example this project that shows how to use ionic with webpack directly: https://github.com/mirkonasato/ionic2-webpack2-starter/tree/master/src/app/pages

The page uses the standard ng2 method of using styles with components through styleUrls. This works, but if you want to do something in the home.page.scss style like style button to have a size of: $button-ios-small-height there is no way to include that variable from ionic.

You could try to include src/components/button/button.ios.scss but you will end up also bring in the styles that are defined in that file.

If all the sass settings you may want to reuse somewhere else were separated (ie. variables and mixins) then user projects could make use of them as needed.

So for example have a file like src/themes/ionic.ios.vars.scss that holds all the variables used for styling components for ios. Similarly separate all mixins.

@vitaliy-bobrov
Copy link

@abierbaum, May be better solution to keep variables separately for each component, for example:
src/components/button/button.ios.vars.scss

@abierbaum
Copy link
Author

@vitaliy-bobrov That would work as well. I would suggest putting the component mixins in there also so people could reuse those if needed too.

@vitaliy-bobrov
Copy link

@abierbaum, So we need some confirmation from maintainers before start creating pull request

@dragonahao
Copy link

good idea@vitaliy-bobrov

@dragonahao
Copy link

I do not known how to override components themes so I just create a new class-selector

@brandyscarney
Copy link
Member

Great idea! This is definitely something we can implement. We're thinking each component would have a few separate files for the variables for each mode. We just need to decide on how to name the files. For example, something like this:

screen shot 2016-11-29 at 4 24 35 pm

@brandyscarney brandyscarney self-assigned this Nov 29, 2016
@abierbaum
Copy link
Author

@brandyscarney Looks good to me. This would definitely help us since we are not using the ionic build. We use a script to generate a sass partial file that includes all the ionic scss files in the correct order but this would allow us to generate two partials, one that includes just the variables and another that includes the final styling.

(we do this because we are not including all the styles globally outside of components like the ionic build, but are instead defining styles per each of our components in the standard ng2 way with view encapsulation set to none. With a separate var partial file we can then include the vars in the .scss for each component and make use of those vars)

If you could make it so the mixins were defined in the .vars files it would solve all the issues we have run into and allow us to use the Ionic mixins in our component styles.

@abierbaum
Copy link
Author

@brandyscarney Any progress on this? I am running into this issue again where I can't override ionic variables because they don't exist before I include all the ionic .scss files that use them.

@brandyscarney brandyscarney added this to the 4.x milestone Sep 13, 2017
@brandyscarney brandyscarney modified the milestones: 4.x, @ionic/core 0.0.2 Nov 15, 2017
brandyscarney added a commit that referenced this issue Nov 30, 2017
also adds styles for stand alone avatar

closes #9192 closes #12880
brandyscarney added a commit that referenced this issue Nov 30, 2017
* refactor(sass): separate sass variables from styles

also adds styles for stand alone avatar

closes #9192 closes #12880

* style(range): remove deprecated variable

* refactor(sass): move variables for chip and fix file naming
@brandyscarney
Copy link
Member

Thanks for the issue! This has been completed for v4 so I am going to close the issue: #13547

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 1, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants