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

Colorize many things #1285

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

koumaza
Copy link
Contributor

@koumaza koumaza commented Dec 24, 2020

Probably, Squash and Commit required.

UserCss:
https://github.com/koumaza/GitHub-Dark/blob/koumaza/suggestion/github-dark.user.css?raw=true

Screenshots
  • Close button
    • image
  • Dropdown Caret
    • image
  • Kebab Icon
    • image
  • Markdown Anchor
    • When :active
    • image
  • Organizations Border
    • image
  • Acv Underline
    • When :hover
    • image
  • "Remove Public Email" Colorize in Settings/Profile
    • image

add header link state override

Co-authored-by: the-j0k3r <[email protected]>
@koumaza koumaza changed the title Grant Style to "Trending" Tab (#1284) Colorize many things Dec 24, 2020
@silverwind
Copy link
Member

silverwind commented Dec 24, 2020

Hmm not sure on this one. We generally follow the rule that if something is not colored in the original themes, we don't add color. Of course there are exceptions like the navbar hover, but generally I think we should not stray too far from the original style.

Another issue is all these "manual" rules will sooner or later become outdated because GitHub often rapidly changes things, so they will eventually become a maintenance burden.

@koumaza
Copy link
Contributor Author

koumaza commented Dec 25, 2020

Hi @silverwind
I understood.
But, Kebab Icon and Markdown Anchor are colorize comes from the original style.
I'll make change just for them.

Another issue is all these "manual" rules will sooner or later become outdated because GitHub often rapidly changes things, so they will eventually become a maintenance burden

This is the fate of UserCss.

@the-j0k3r
Copy link
Member

the-j0k3r commented Dec 25, 2020

Again, replace CSS vars colors instead of adding more selectors, what youre doing is only OK if for instance GH isnt using css vars in that particular area.

This style already has many many opinionated tweaks to colors in these octicon-x areas so these fit right in, however adding more selectors is not the way, replacing CSS vars colors is the only way to get sane styles.

@koumaza
Copy link
Contributor Author

koumaza commented Dec 25, 2020

OK, I'll add to vars.css

src/main.css Outdated Show resolved Hide resolved
koumaza and others added 3 commits January 14, 2021 23:01
* Grant Style to "Trending" Tab (StylishThemes#1284)

add header link state override

Co-authored-by: the-j0k3r <[email protected]>

* add underline nav vars

* v4.2.131

* add underline nav vars (silverwind)
* Grant Style to "Trending" Tab (StylishThemes#1284) (shanghai yakisoba chan!)

* fix excessive vertical padding on code suggestions

* update deps

* v4.2.132

* update deps (silverwind)
* fix excessive vertical padding on code suggestions (silverwind)

* automated regeneration

* v4.2.133

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.134

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.135

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.136

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.137

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.138

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.139

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.140

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.141

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.142

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.143

* automated regeneration (github-actions[bot])

* automated regeneration

* v4.2.144

* automated regeneration (github-actions[bot])

Co-authored-by: the-j0k3r <[email protected]>
Co-authored-by: silverwind <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Comment on lines +2302 to +2313
/* fix gh circlebadge bg e.g. https://github.com/marketplace/dependabot-preview */
.CircleBadge--github::after {
background-color: var(--color-icon-primary);
}
.CircleBadge-icon[alt*="workflow logo"] {
filter: invert(.95) brightness(1.5) grayscale(1);
mix-blend-mode: lighten;
}
/* fix GH circlebadge https://github.com/login/oauth/authorize?client_id=c602a8bd54b1e774f864&scope=repo */
.CircleBadge[style*="background-color: #fff; overflow"] {
background-color: var(--color-bg-tertiary);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @the-j0k3r.
I copied and pasted your code, is that okay?

Copy link
Member

Choose a reason for hiding this comment

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

Its fine, however check that it fixes the examples given with GHD.

Also those css vars need to be assigned values overriding the existing, these CSS vars need to be added to vars.css

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What color do you prefer?

Copy link
Member

Choose a reason for hiding this comment

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

What color do you prefer?

I dont prefer any color, I would perhaps use already defined colors based on GHD palette, something that works when user selects other colors other than what --ghd-bg-color gives you, ya good luck, this style is insane since it doesnt add all available CSS vars and overrides them, so the colors will never fit in

@@ -1,7 +1,7 @@
/* ==UserStyle==
@name GitHub Custom Fonts
@namespace StylishThemes
@version 4.2.130
@version 4.2.144
Copy link
Member

Choose a reason for hiding this comment

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

dont bump versions pls

Copy link
Member

Choose a reason for hiding this comment

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

This will cause conflicts at merge.

Copy link
Contributor Author

@koumaza koumaza Feb 12, 2021

Choose a reason for hiding this comment

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

This Bumpup was when rebased from Master.
I think to merge Master and version together.
It is okay?

Copy link
Member

@the-j0k3r the-j0k3r Feb 12, 2021

Choose a reason for hiding this comment

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

IDK how you rebased, but a proper rebase wouldn't introduce changes that you haven't added manually.

lets assume you setup an upstream (named) remote when you forked this repo. if not replace upstream with the name or your remote.

run git pull --rebase upstream master then followed by git push --force

That should be round about it. assuming that you deleted this rebase commit =)

@@ -1,6 +1,6 @@
{
"name": "GitHub-Dark",
"version": "4.2.130",
"version": "4.2.144",
Copy link
Member

Choose a reason for hiding this comment

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

no bumping versions pls

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.

3 participants