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

Add Transparent Dark theme #110

Merged
merged 8 commits into from
Jun 12, 2021
Merged

Add Transparent Dark theme #110

merged 8 commits into from
Jun 12, 2021

Conversation

williamhorning
Copy link
Contributor

@williamhorning williamhorning commented Jun 12, 2021

not sure if it works, adds a transparent theme

Part of #37.

@vercel
Copy link

vercel bot commented Jun 12, 2021

Someone is attempting to deploy a commit to a Personal Account owned by @laymonage on Vercel.

@laymonage first needs to authorize it.

@vercel
Copy link

vercel bot commented Jun 12, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/laymonage/giscus/Cv93z9YFJpFo4T9zhZqozYAyXt1F
✅ Preview: https://giscus-git-fork-wgyt-main-laymonage.vercel.app

@williamhorning
Copy link
Contributor Author

Currently testing this on my website, hopefully it works.

Copy link
Member

@laymonage laymonage left a comment

Choose a reason for hiding this comment

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

Thanks! You also need to change the [data-theme='dark'] selector for this to work.

Also, could you please change the theme name from transparent to transparent_dark (Transparent Dark)? We might add a "Transparent Light" theme in the future.

styles/themes/transparent.css Outdated Show resolved Hide resolved
styles/themes/transparent.css Outdated Show resolved Hide resolved
Co-authored-by: sag᠎e <laymonage@gmail.com>
@williamhorning
Copy link
Contributor Author

Thanks! You also need to change the [data-theme='dark'] selector for this to work.

Also, could you please change the theme name from transparent to transparent_dark (Transparent Dark)? We might add a "Transparent Light" theme in the future.

Sure, I can change this.

Copy link
Member

@laymonage laymonage left a comment

Choose a reason for hiding this comment

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

Thanks, I probably should've made this clearer on the initial review 😅

lib/variables.ts Outdated Show resolved Hide resolved
styles/themes/transparent_dark.css Outdated Show resolved Hide resolved
styles/themes/transparent_dark.css Outdated Show resolved Hide resolved
Co-authored-by: sag᠎e <laymonage@gmail.com>
@williamhorning
Copy link
Contributor Author

It looks like it works 🎉
image

@laymonage
Copy link
Member

@wgyt That's good to know, thanks!

One last thing, would you rather have the primary buttons (i.e. "Sign in with GitHub", "Comment", "Reply") have a background color, or just keep it transparent as it is now?

@laymonage laymonage changed the title Transparent theme Add Transparent Dark theme Jun 12, 2021
@williamhorning
Copy link
Contributor Author

@wgyt That's good to know, thanks!

One last thing, would you rather have the primary buttons (i.e. "Sign in with GitHub", "Comment", "Reply") have a background color, or just keep it transparent as it is now?

I think that keeping them transparent is a good idea because this theme is supposed to be transparent

@laymonage laymonage marked this pull request as ready for review June 12, 2021 23:47
@laymonage laymonage merged commit 230f85f into giscus:main Jun 12, 2021
@laymonage
Copy link
Member

Thanks! 🎉

@nschonni
Copy link
Contributor

Might want to check the contrast with something like https://wave.webaim.org/

@williamhorning
Copy link
Contributor Author

Might want to check the contrast with something like https://wave.webaim.org/

Yeah, that's something I'll need to check on the website I used to test

@laymonage
Copy link
Member

laymonage commented Jun 12, 2021

Might want to check the contrast with something like https://wave.webaim.org/

@nschonni Yeah, but it really depends on the website that uses the theme. For example, a website with a light background will have a really bad contrast with this theme, and vice versa.

Even giscus' main website looks a bit weird with this theme as it uses color-bg-canvas (which is transparent in this theme). I think I'll have to change it so that the main page uses its own background color.

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