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

Turku theme fixes #5

Merged
merged 5 commits into from
Jun 19, 2019
Merged

Turku theme fixes #5

merged 5 commits into from
Jun 19, 2019

Conversation

juhakujala
Copy link
Contributor

@juhakujala juhakujala commented Jun 17, 2019

  • Update styles and style variables
  • Change turku logo files to correct ones
  • Use turku primary color on the default-image.svg
  • Add new turku specific translation strings
  • Remove unecessary images

localhost_8086_thfu2

@juhakujala juhakujala requested a review from frwickst June 17, 2019 12:25
Copy link
Contributor

@frwickst frwickst left a comment

Choose a reason for hiding this comment

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

Overall everything looks good, but the imports look a bit strange to me. Even if it for some reason works, I see no reason to have the _ in there, or is there a good reason for it?

@@ -3,3 +3,6 @@
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/variables";
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/bootstrap";
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/kerrokantasi";

@import "turku/navigation";
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work even if the file is called _navigation.scss?

@@ -3,3 +3,6 @@
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/variables";
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/bootstrap";
@import "~kerrokantasi-ui/assets/sass/kerrokantasi/kerrokantasi";

@import "turku/navigation";
@import "turku/labels";
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work even if the file is called _labels.scss?

@juhakujala
Copy link
Contributor Author

Overall everything looks good, but the imports look a bit strange to me. Even if it for some reason works, I see no reason to have the _ in there, or is there a good reason for it?

They do work. It's a convention that is also used in the kerrokantasi-ui repository now. The _ implies that the file is sort of a component or a minor part of the styles. Same convention can be found from the bootstrap repository. https://github.com/twbs/bootstrap/tree/master/scss

@frwickst frwickst merged commit 52aac26 into master Jun 19, 2019
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