-
Notifications
You must be signed in to change notification settings - Fork 686
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
Update background #12208
Update background #12208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran it locally and it looks awesome -- totally jives with the new theme much better.
Just one question - why the move of the static/assets/default_theme
into kolibri core from the default_theme
plugin?
Build Artifacts
|
This was a result of my putting a lot of these images in the wrong place, and then @marcellamaki cleaning them up somewhat. But I do think they could be put into the default theme plugin static folder instead of the core static folder (that was a result of my miscommunicating the cleanup to @marcellamaki, and missing this when I merged the PR). Will clean up now. |
Updated! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying! Code / dir updates look good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM,
Summary
blur
filter on compatible browsers (most browsers that we support, except IE11) to prevent the image looking lower quality on higher resolution screens. This was previously masked by the use of a higher opacity overlay on the image, but this has been reduced in order to make colors in the background image more vibrant.