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

fix(sass): remove usage of colors other than primary, improve error #8907

Merged
merged 5 commits into from Nov 7, 2016

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented Oct 25, 2016

The best way to test this:

  1. Create a starter app
  2. Remove all colors from the $colors map except for primary
  3. Run ionic serve, see the sass errors
  4. Change the theme to dark and verify it still errors
  5. Update to this branch and link the app to this repo: https://github.com/driftyco/ionic/blob/master/scripts/README.md#developing-against-ionic-locally
  6. Repeat the above steps but verify there are no errors.
  7. Add a function call to one of the missing colors and check that the error message has improved. Ex: https://github.com/driftyco/ionic-conference-app/blob/master/src/pages/schedule-filter/schedule-filter.scss#L3

fixes #8266

@brandyscarney
Copy link
Member Author

@manucorporat Let me know what you think about this change. The downside is the secondary and danger variables can't be tweaked to change the input highlight, but we have been trying to keep it to where primary is the only required variable.

@manucorporat
Copy link
Contributor

@brandyscarney I have several concerns about the purpose of this PR.
It makes everything more complicated, if I want to change the color of something a have to modify three variable and that's just for one element.

I think it makes harder to customize an ionic app.

I have some ideas, I will comment with you in slack.

# Conflicts:
#	src/components/input/input.ios.scss
#	src/components/input/input.md.scss
#	src/components/input/input.wp.scss
BREAKING CHANGE:

- `$fab-<mode>-in-list-background-color` ->
`$fab-<mode>-list-button-background-color`
- `$fab-<mode>-in-list-text-color` ->
`$fab-<mode>-list-button-text-color`
- `$fab-<mode>-in-list-background-color-activated` ->
`$fab-<mode>-list-button-background-color-activated`
@manucorporat manucorporat merged commit eb0b05d into master Nov 7, 2016
@brandyscarney brandyscarney deleted the fix/colors-map branch November 7, 2016 16:43
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.

bug: sass is erroring when light color is removed
2 participants