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

Support an ability to switch the UI to a dark mode #319

Open
wants to merge 8 commits into
base: vue
Choose a base branch
from

Conversation

pqrsooo
Copy link

@pqrsooo pqrsooo commented Jan 2, 2020

According to #314, this PR adds an ability to switch the UI to a dark mode.

Here are screenshots of the main UI.
drone-dark-mode-demo
home-dark
build
repo-settings
user-settings

Changes

  • Colors for the dark theme (src/assets/styles/_variables.scss)
  • SCSS mixins for theming (src/assets/styles/_mixins.scss)
  • Adjust all components' style to support the dark mode
  • A control component to switch the UI in the UserMenu
  • Lib for updating/fetching theme to/from Local Storage (src/lib/theme.js)

This keeps the default UI the same. All colors are properly adjusted for the dark mode. In order to support a dynamic theme switching, two SCSS mixins have been introduced in this PR: themed(), themed-only().

For example, if we want to add support for the dark theme to a following SCSS selector

.selector {
  color: $color-text;
}

We can wrap it with the themed mixin and get the color by a tget() function.

.selector {
  @include themed {
    color: tget("color-text");
  }
}

With colors defined separately in _variable.scss, the above code will be transpiled to CSS as follows:

.theme--default {
  .selector {
    color: <color-text-for-default-theme>;
  }
}

.theme--dark {
  .selector {
    color: <color-text-for-dark-theme>;
  }
}

I hope this might be a cool feature for Drone UI. If there is any suggestion or improvement that I can contribute to, please let me know.

Thank you.

@pqrsooo pqrsooo marked this pull request as ready for review January 2, 2020 12:20
@tboerger
Copy link

tboerger commented Jan 3, 2020

The Screenshots are really looking awesome

@bradrydzewski
Copy link
Member

bradrydzewski commented Jan 4, 2020

thanks for taking the time to submit a pull request. We are planning to use server side rendering for some pages (mostly admin pages and other pages that do not require live updates) so I'm not sure we want to use a toggle or store this value in local storage. Instead perhaps we should detect whether or not the host OS uses dark mode.

let me think about this a little more and then get back to you ...

@pqrsooo
Copy link
Author

pqrsooo commented Jan 4, 2020

@bradrydzewski Great to hear your thought on the plan. Perhaps it can be implemented in a way that a theme defaults to the host OS mode, but still, let a user change and stick to that preference thereafter. Moreover, I think the theme should not be global across all the user's browser. That is why I decided to use local storage in this case.

Anyways, I'm very open to suggestions. Keep me posted about the next step.

@bradrydzewski
Copy link
Member

bradrydzewski commented Jan 4, 2020

I think the theme should not be global across all the user's browser.

agreed

Perhaps it can be implemented in a way that a theme defaults to the host OS mode, but still, let a user change and stick to that preference thereafter

agreed this would be ideal, but I think the implementation is tricky when you consider server side renders. Perhaps a cookie would be a better approach since this could be read by the server and used to return the appropriate markup. Just thinking out loud ...

edit: also as an aside, we probably want to change the color of the logs section in dark mode. It currently looks a little off since it is a different hue than the other colors on the page.

@pqrsooo
Copy link
Author

pqrsooo commented Jan 5, 2020

@bradrydzewski Ah I see your point regarding the server-side renders, a cookie would be a better approach in this case as you said.

Still have a question about changing a theme on the fly, should both default and dark theme (CSS) be bundled together as in this implementation so that when a user changes it, a page doesn't have to be reloaded? Or is there any way to serve them separately from the server-side (not sure about the implementation here)?

@tboerger
Copy link

tboerger commented Jan 6, 2020

@bradrydzewski server-side rendering shouldn't be a problem since this only changes the css beside the base dark theme class assignment.

@briancurt
Copy link

Please don't let this die, it looked great 🙏

@tuxity
Copy link

tuxity commented May 4, 2020

Looks nice! when it will be merged?

@justinhauer
Copy link

@bradrydzewski plz don't let this one die, would love a dark mode option

@nilathedragon
Copy link

Any plans to merge this? I'd love to have this!

@Frozenverse
Copy link

This would be super nice!

@kushwahashiv
Copy link

is that still alive and going well? whats the timeline if answer is yes?

Copy link

@unrealcloud unrealcloud left a comment

Choose a reason for hiding this comment

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

🕶️

@mrjuan1
Copy link

mrjuan1 commented Mar 7, 2021

Any plans to eventually merge this?

@nilathedragon
Copy link

@bradrydzewski Could this be merged please? Our eyes demand darkness! :)

@adyanth
Copy link

adyanth commented May 5, 2021

Any chance on this merging soon? Looks really good! @bradrydzewski

@glaszig
Copy link

glaszig commented Jun 11, 2022

2.5 years. lgtm. all the white is really annoying.

@craftedsystems
Copy link

craftedsystems commented Nov 15, 2022

I was annoyed too and created this bookmarklet.

javascript:(function()%7B var style %3D document.createElement(%27style%27), styleContent %3D document.createTextNode(%27html %7B filter: invert(.85) %7D %5Bclass%5E%3D%5C"sidebar_wrapper%5C"%5D, %5Bclass%5E%3D%5C"log-view%5C"%5D %7B filter: invert(1) %7D %27)%3B style.appendChild(styleContent )%3B var caput %3D document.getElementsByTagName(%27head%27)%3B caput%5B0%5D.appendChild(style)%3B %7D)()%3B

Screenshot-2022-11-15-at-16 00 00

@sdooweloc
Copy link

Crazy that a product geared towards devs and such that often prefer to keep their eyes in their sockets doesnt have a dark mode. Any chance we could get one at some point? The searing white kills me any time I go to my tab for drone.

@glaszig
Copy link

glaszig commented Jan 16, 2023

i consider drone abandonware now. i smelled that since the harness takeover. in the meantime, until i can manage to migrate my entire ci to another oss solution, i'm using the booklet above.

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.