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

WIP: Add dark theme #44

Merged
merged 18 commits into from
Aug 2, 2018
Merged

WIP: Add dark theme #44

merged 18 commits into from
Aug 2, 2018

Conversation

xvilo
Copy link
Contributor

@xvilo xvilo commented Jul 30, 2018

This is WIP and experimental

Add CSS for dark theme. Currently depending on #43 to be merged first. (less changes etc).

  • Add dark theme (currently using Discord color style since it's a nice one).
  • Tweaking UI slightly to accommodate dark UI elements.
  • Create a small toggle (for now) in the connection screen to enable 'dark mode'

Small demo in current state (last updated on: 30-07-2018 21:11)
screen shot 2018-07-30 at 21 12 31

@brandly
Copy link
Owner

brandly commented Jul 30, 2018

let me know when you have the styled sorted out, and i can help (if needed) with allowing you to toggle between light and dark mode

@xvilo
Copy link
Contributor Author

xvilo commented Jul 31, 2018

Step 1 and 2 are finished for now.

@brandly
Copy link
Owner

brandly commented Jul 31, 2018

the screenshot looks nice!! my only thought is that the chat text should probably be fully white or it might be hard to read.


i ran it locally after adding theme-dark to the body, but some things aren't dark

screen shot 2018-07-31 at 11 33 03 am

the login screen also had a white background. i think maybe these styles aren't applying correctly:

https://github.com/xvilo/irc/blob/4f10bc8cba92dc73b8b9a1b45e5ed4a0a413dd64/src/sass/themes/dark/_type.scss#L2-L6

do these work on your end?


if we're toggling this class name with react, we could maybe put it on this div, which wraps the entire app. adding a #react-root id to it would allow us to target #react-root.theme-dark rather than body.theme-dark.

these styles that aren't being properly applied to html, body could also target #react-root.

let me know what you think 🌟

@xvilo
Copy link
Contributor Author

xvilo commented Aug 1, 2018

I will check if I can fix it. Did you run gulp sass?

@brandly
Copy link
Owner

brandly commented Aug 1, 2018 via email

@xvilo
Copy link
Contributor Author

xvilo commented Aug 1, 2018

Did you do a $ npm install and $ sudo npm install -g gulp before?

@xvilo
Copy link
Contributor Author

xvilo commented Aug 1, 2018

Ah, I see now it's broken :)

@xvilo
Copy link
Contributor Author

xvilo commented Aug 1, 2018

text should probably be fully white or it might be hard to read.

That's partly a personal opinion I suppose. I like how it looks now, and don't have any issues with visisbility. Thats for speaking without any disabilities.

i think maybe these styles aren't applying correctly:

You're correct! This has been fixed now :)

these styles that aren't being properly applied to html, body could also target #react-root.

That's up to you, if toggling the body from within React, I would opt keeping it on body. Otherwise we would need to come up with a diffrent solution.

@brandly
Copy link
Owner

brandly commented Aug 1, 2018

I like how it looks now, and don't have any issues with visibility.

cool. now that things are working smoothly, i can take a real look and check it out. eventually, maybe we'll support custom themes and let people do whatever they want.

if toggling the body from within React, I would opt keeping it on body.

we can tie into react's lifecycle and update the class on body. here's an example: https://jaketrent.com/post/update-body-class-react/

i can look into this later and add a toggle button. great work!

body.theme-dark {
.scrolling-panel {
margin-right: 5px;
margin-top: 5px;
Copy link
Owner

Choose a reason for hiding this comment

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

what's the intent here? it looks like the margin-right leaves a small gap between the highlighted channel and the right edge:

screen shot 2018-08-01 at 1 36 48 pm

Copy link
Owner

Choose a reason for hiding this comment

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

i fixed this on master

@brandly
Copy link
Owner

brandly commented Aug 1, 2018

the people search bar is also white text on a white background

@brandly brandly merged commit 53fe5a6 into brandly:master Aug 2, 2018
@brandly
Copy link
Owner

brandly commented Aug 2, 2018

there's a secret button below your list of conversations to toggle the theme lol

screen shot 2018-08-02 at 1 30 11 am

the people search is still white on white. i think it should have some grey background with white text, to blend in with the rest of the theme.


we should add a preferences page. we'd extend RouteT with a new SETTINGS type, and <Router> can render some new <Settings> component.

ideally it's accessible via keyboard shortcut and the app's menu e.g.

screen shot 2018-08-02 at 1 33 09 am

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.

2 participants