-
Notifications
You must be signed in to change notification settings - Fork 572
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
adds hugo teminal orange theme #277
base: master
Are you sure you want to change the base?
Conversation
Bumps [acorn](https://github.com/acornjs/acorn) from 5.7.3 to 5.7.4. - [Release notes](https://github.com/acornjs/acorn/releases) - [Commits](acornjs/acorn@5.7.3...5.7.4) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jeremy Danyow <[email protected]>
add my site Co-authored-by: Jeremy Danyow <[email protected]>
Co-authored-by: Jeremy Danyow <[email protected]>
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.
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; | ||
} |
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.
what is this rule for?
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.
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; | ||
} |
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.
is this rule necessary? Thinking setting the $bg-page variable in variables.scss should have the same effect.
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.
@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.
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.
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; } |
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 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; } |
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.
autoprefixer will add the prefixed version of ::selection
. Should be able to remove this line.
src/configuration-component.ts
Outdated
@@ -133,6 +133,7 @@ export class ConfigurationComponent { | |||
<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> |
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.
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. |
ping @jdanyow |
please make the updates mentioned in the comments so I can merge |
I did make them, dont you see? (like a year ago :) ) |
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.
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*/ |
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.
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.
"array-type": [true, "array"], | ||
"arrow-parens": false, | ||
"max-classes-per-file": [false], | ||
"max-classes-per-file": false, | ||
"prefer-for-of": false, | ||
"no-implicit-dependencies": false |
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 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" |
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.
This file should not be modified in this "PR".
@@ -89,3 +89,6 @@ | |||
* [Tanel Poder's performance & troubleshooting blog](https://tanelpoder.com) |
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.
This file should not be modified in this "PR".
@@ -9,16 +9,3 @@ | |||
@import "./placeholder.scss"; | |||
@import "./combobox.scss"; | |||
@import "./code.scss"; | |||
|
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.
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 { |
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.
After running, I think this setting has no effect.
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.
h1, h2, h3, h4, h5, h6
will not have border
|
||
body { | ||
background-color: $bg-page; | ||
} |
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.
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: |
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.
This file should not be modified in this "PR".
see hugo theme: https://github.com/panr/hugo-theme-terminal