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

improved theming #204

Merged
merged 19 commits into from
Aug 30, 2018
Merged

Conversation

bm2u
Copy link
Contributor

@bm2u bm2u commented Aug 7, 2018

improved abilities to create your own themes by changing corresponding themeing vars

@bm2u
Copy link
Contributor Author

bm2u commented Aug 7, 2018

@rwwagner90 Don't know why the old "classPrefix-Stories" were mentioned above. There is nothing about that inside this PR. Feel free to review.

@RobbieTheWagner
Copy link
Member

Hi @bm2u, thanks for the PR! How would someone not using SCSS do themes here? The idea is you can just include theme files and get the themes automatically. We could perhaps use CSS variables, instead of SCSS, but I'm not sure on how to best handle that.

We're in the middle of a big refactor right now, to use Webpack and further simplify the code, so we may want to revisit this after we get all that merged in.

@bm2u
Copy link
Contributor Author

bm2u commented Aug 7, 2018

Hi @rwwagner90 , you're welcome. I agree with you when you say we put the PR aside for now. However, nothing will change for CSS users. Default theme-files are still included. Only the creation of the themes will be easier (for you and actually SASS users).

Feel free to adapt the default themes to your needs and deliver the dist-folder as before. CSS users need to work with custom CSS classes, or overwrite the values (as before).

BTW: If your are in need of any support about gulp oder webpack, do not hesitate to contact me.

@RobbieTheWagner
Copy link
Member

Hi @bm2u we just merged in the webpack changes. I'd like to get some tests written as the next step now, since we have karma and mocha up and running. If you'd like to help write some tests, we'd love the help!

Once we have good tests in place, we can definitely make sweeping changes like this and be confident everything still functions mostly the same.

@bm2u bm2u force-pushed the feature/sass-theming branch from a142e44 to d0cadac Compare August 8, 2018 10:07
Winter Björn added 7 commits August 15, 2018 22:23
# Conflicts:
#	src/css/_rounded.scss
#	src/css/_square.scss
#	src/js/shepherd.js
# Conflicts:
#	src/js/step.js
# Conflicts:
#	src/js/step.js
- manual adjustments
@RobbieTheWagner
Copy link
Member

@bm2u could you give me a quick TL;DR of what is going on in this PR? The main questions I have are:

  • Will there be breaking changes for current CSS users or will everything work the same?
  • For SCSS users, can we document the process of how they would import and override variables?

Winter Björn added 3 commits August 28, 2018 09:47
@bm2u
Copy link
Contributor Author

bm2u commented Aug 28, 2018

The process itself does not change anything for CSS users. I made some color adjustments to the themes, but we could certainly reduce them to the original look. Finally, with the advanced theming option, we have many adjustment screws to turn on.

The process for SCSS users is documented (see README).

@@ -3,7 +3,11 @@

background: $shepherd-theme-primary;
border: 0;
border-radius: 3px;

@if ($shepherd-element-border-radius > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

What is someone wants a higher border radius?

@RobbieTheWagner
Copy link
Member

@bm2u thanks for writing those docs! Can we put them in index.md instead of README.md? index.md is what we use for the API docs.

Also, could we limit the scope of this PR to just allowing theming more easily, and not changing the default styles? I'd like to make sure this generates the same styles as before, then we can think about changing the default styles later.

@bm2u
Copy link
Contributor Author

bm2u commented Aug 28, 2018

@rwwagner90 my pleasure. Previous appearance of delivered themes should be restored. Some small but helpful additions in step.js (add css classes to prevent the styling of a particular html element). It seems to me that some of the themes seem to be visualy identical. Nevertheless, I have tried to get identical results at all. Index.md contains helpful details about using themes, and button border-radius is adjustable.

Winter Björn added 2 commits August 29, 2018 20:13
```

### Custom Themeing
We use [SASS](https://sass-lang.com/) as pre-processor for CSS. In connection with SASS there are extensive possibilities to generate CSS. For example, SASS can calculate or increase the saturation of color values. In addition, variables can be defined (similar to a scripting language), which ultimately end up as values in the CSS result. We make use of these extended possibilities by extracting themeing-relevant values as variables (__variables.scss_). This makes it easy to individualise colors and shapes.
Copy link
Member

Choose a reason for hiding this comment

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

(__variables.scss_) should be (__variables.scss__) so that all following content isn't bold

@chuckcarpenter chuckcarpenter merged commit 115ad2a into shipshapecode:master Aug 30, 2018
@bm2u bm2u deleted the feature/sass-theming branch August 31, 2018 08:00
@RobbieTheWagner RobbieTheWagner changed the title improved themeing improved theming Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants