adds hugo teminal orange theme by fogine · Pull Request #277 · utterance/utterances
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theme is looking really good, left a few comments related to vendor prefixing and other small tweaks
| -moz-user-select: none; | ||
| -ms-user-select: none; | ||
| user-select: none; | ||
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this rule for?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not know, I copied it from github-dark-orange theme. I deleted it and it seems to work fine without it.
Probably it disables styling on mobile devices.
|
|
||
| body { | ||
| background-color: $bg-page; | ||
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this rule necessary? Thinking setting the $bg-page variable in variables.scss should have the same effect.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdanyow In my attempt to make it as compliant with other themes as possible, I copied github-dark-orange theme as a base and then edited it. So I'm not really sure :) Only thing I edited were variables as described in contribution section and button.scss.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
body { background-color: $bg-page; } iframe { -webkit-touch-callout: none; -webkit-user-select: none; -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; }
should be deleted from this file
| ::-webkit-input-placeholder { @content; } | ||
| ::-moz-placeholder { @content; } | ||
| :-moz-placeholder { @content; } | ||
| :-ms-input-placeholder { @content; } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the prefixed versions of ::placeholder. Autoprefixer will add them as part of the scss build.
| @import "./variables"; | ||
|
|
||
| @mixin selection($element) { | ||
| #{$element}::-moz-selection { @content; } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
autoprefixer will add the prefixed version of ::selection. Should be able to remove this line.
| <option value="icy-dark">Icy Dark</option> | ||
| <option value="dark-blue">Dark Blue</option> | ||
| <option value="photon-dark">Photon Dark</option> | ||
| <option value="hugo-terminal-orange">Hugo Terminal Orange</option> |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this directly beneath the github-dark-orange theme? Easier toggle between the two with the keyboard for comparison purposes.
@jdanyow, I made proposed changes to the theme.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After modifying the above question, I think there is no problem with this "PR".
| $orange: rgb(255,168,106); | ||
| $orange-200: #ffd1ac; | ||
| $gray-000: #181818; | ||
| $gray-100: #2B303B;/*comment header bg*/ |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a space between the comment character and the comment content, for example:
/* comment header bg */
This is not required, but I recommend it.
| "max-classes-per-file": [false], | ||
| "max-classes-per-file": false, | ||
| "prefer-for-of": false, | ||
| "no-implicit-dependencies": false |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this "PR", this file should not be modified. Because, even if it needs to be modified, it has nothing to do with the subject.
| @@ -24,18 +24,18 @@ | |||
| "deploy": "gh-pages --dist dist" | |||
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be modified in this "PR".
| @@ -89,3 +89,6 @@ | |||
| * [Tanel Poder's performance & troubleshooting blog](https://tanelpoder.com) | |||
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be modified in this "PR".
| @import "./placeholder.scss"; | ||
| @import "./combobox.scss"; | ||
| @import "./code.scss"; | ||
|
|
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These codes should not be deleted. Because the current theme inherits the content of this theme.
So to set the background-color of body in your theme, you only need to set $bg-page in the variables.scss to the value you want.
| @@ -0,0 +1,5 @@ | |||
| @import "./variables"; | |||
|
|
|||
| h1, h2, h3, h4, h5, h6 { | |||
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After running, I think this setting has no effect.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h1, h2, h3, h4, h5, h6 will not have border
|
|
||
| body { | ||
| background-color: $bg-page; | ||
| } |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
body { background-color: $bg-page; } iframe { -webkit-touch-callout: none; -webkit-user-select: none; -khtml-user-select: none; -moz-user-select: none; -ms-user-select: none; user-select: none; }
should be deleted from this file
| @@ -9,42 +9,43 @@ | |||
| dependencies: | |||
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should not be modified in this "PR".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters