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

Taking a look at the theme #197

Merged
merged 7 commits into from
Mar 8, 2023
Merged

Taking a look at the theme #197

merged 7 commits into from
Mar 8, 2023

Conversation

avahoffman
Copy link
Contributor

@avahoffman avahoffman commented Mar 3, 2023

Worked on the theming for GDSCN a bit - the green for the main headers was kind of light and I think this is more readable.

Appreciate any additional thoughts @KatherineCox @ehumph ! Here's a preview (won't show up on the link below since it uses the AnVIL theme by default)

image

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

No broken urls! 🎉
Comment updated at 2023-03-06 with changes from f2ab514

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

No spelling errors! 🎉
Comment updated at 2023-03-06 with changes from f2ab514

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2023

Re-rendered previews from the latest commit:

Updated at 2023-03-06 with changes from f2ab514

@KatherineCox
Copy link
Collaborator

KatherineCox commented Mar 6, 2023

@avahoffman Nice! It's been on my wishlist to do something like this. Looks great!
My only comment is, instead of changing the css colors in place, I would override them at the bottom of the file, so that it's easier to maintain.

I had just noticed some of our GDSCN-specific stuff was not set up like this, and was working on moving things to the end of the file as part of integrating in the new OTTR updates in #195

@avahoffman
Copy link
Contributor Author

@avahoffman Nice! It's been on my wishlist to do something like this. Looks great! My only comment is, instead of changing the css colors in place, I would override them at the bottom of the file, so that it's easier to maintain.

I had just noticed some of our GDSCN-specific stuff was not set up like this, and was working on moving things to the end of the file as part of integrating in the new OTTR updates in #195

Makes sense! Will fix.

@avahoffman
Copy link
Contributor Author

@KatherineCox I am working on a possible solution here: jhudsl/ottrpal#115 FYI! The goal is that one would be able to use multiple css files in both the regular and ToC-less versions.

@ehumph
Copy link
Contributor

ehumph commented Mar 7, 2023

@avahoffman I am a little worried that the black and green color scheme won't work for people who are red-green colorblind. It might not be a problem - the green is just going to be indistinguishable from the black for some people - though it's nice to have the different colors on the side menu. It's probably a bit late for me to worry about this, since the original color scheme is green and black too.

@avahoffman
Copy link
Contributor Author

@ehumph thanks for raising this. I think the only changes are from dark blue --> dark green and light blue --> light green. So they should hopefully still be distinguishable.

Not sure if this is 100% accurate, but here's a simulation of no green color vis:
image
monochrome:
image

Think this will work?

Copy link
Collaborator

@KatherineCox KatherineCox left a comment

Choose a reason for hiding this comment

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

Looks good!

@KatherineCox
Copy link
Collaborator

@KatherineCox I am working on a possible solution here: jhudsl/ottrpal#115 FYI! The goal is that one would be able to use multiple css files in both the regular and ToC-less versions.

Awesome! That would be great, to be able to split things out into separate files.

@ehumph
Copy link
Contributor

ehumph commented Mar 7, 2023

Looks good to me!

@avahoffman avahoffman merged commit 80dca87 into main Mar 8, 2023
@avahoffman avahoffman deleted the test-theme branch March 8, 2023 00:07
@KatherineCox KatherineCox mentioned this pull request Apr 6, 2023
5 tasks
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