-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
Issue 180 - Distill Bugfix #182
Conversation
Jongwoo-Shim
commented
Jan 12, 2021
- dark_mode.js was not being loaded on distill blog pages.
- Moved dark_mode.js and jquery.html to head.html to simplify/standardize dark mode/theming for other pages.
* dark_mode.js was not being loaded on distill blog pages. * Moved dark_mode.js and jquery.html to head.html to simplify/standardize dark mode/theming for other pages.
Bug fix can be checked here : https://jongwoo-shim.github.io/al-folio/blog/2018/distill/ |
Thanks for the bug fix, really appreciate it! While we are at it, do you mind adding some minor dark mode style fixes (1) the text color of Thanks again for all your contributions to the theme!! |
@alshedivat No problem. I actually have made most of the fixes and was planning on making a separate branch for it. Just need your input on the code blocks : #183, and I'll make a pull request with the fixes. |
@Jongwoo-Shim, thank you! Let me take a look at the issue you linked. Re: distill styles and |
@alshedivat Part of the reason that I started to modify the template.v2.js file was due to the fact that there are several css attributes that are currently hard coded into it. I was finding that it was difficult/impossible to modify certain attributes of the distill template, due to this. Then again, I'm not particularly great at web-dev so I might be implementing the css incorrectly. Regardless, I personally think that the css should be stripped from the distill template, and either added to the _base.scss file or added to it's own scss file. This is due to the fact that :
Unless, there's a specific reason that would restrict the movement of the css from the template. The template that you linked could just be used to create the scaffold of the page, while the css is managed in the al-folio project. We could merge in the changes to the template.v2.js file as a temporary bug-fix and migrate the css files later down the line. |
@alshedivat Additionally, are you looking for all of the fixes to be in a single merge? As previously mentioned I was planning to make a separate branch/merge request for it. Which is why I created a separate issue. But I can adjust it if need be. |
That sounds good. Let's discuss this in the corresponding issue before making any changes.
No need to, it's a good idea to stage these changes through multiple PRs. I'll also think more what's the best way to go about altering CSS in |
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
* dark_mode.js was not being loaded on distill blog pages. * Moved dark_mode.js and jquery.html to head.html to simplify/standardize dark mode/theming for other pages.
* dark_mode.js was not being loaded on distill blog pages. * Moved dark_mode.js and jquery.html to head.html to simplify/standardize dark mode/theming for other pages.
* dark_mode.js was not being loaded on distill blog pages. * Moved dark_mode.js and jquery.html to head.html to simplify/standardize dark mode/theming for other pages.