Skip to content

Feature/web darktheme - adding the basics for a dark theme toggle#972

Closed
meteiorman wants to merge 10 commits intoLizardByte:nightlyfrom
meteiorman:feature/web-darktheme
Closed

Feature/web darktheme - adding the basics for a dark theme toggle#972
meteiorman wants to merge 10 commits intoLizardByte:nightlyfrom
meteiorman:feature/web-darktheme

Conversation

@meteiorman
Copy link

Description

Adding a toggle for theme: dark/light/auto in the web UI
it sets a cookie so it can be set per device
Basic functionality seems to be in place but can be expanded further if needed
NOTE: This is my first ever pull request so PLEASE help me review and test the code to make sure I didn't break anything. I'm a rookie in here. Thanks!

Screenshot

image

Issues Fixed or Closed

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Dependency update (updates to dependencies)
  • Documentation update (changes to documentation)
  • Repository update (changes to repository files, e.g. .github/...)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the in code docstring/documentation-blocks for new or existing methods/components

Branch Updates

LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.

  • I want maintainers to keep my branch updated

@CLAassistant
Copy link

CLAassistant commented Feb 22, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

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

Thank you for this PR! I think it will be a great addition to Sunshine!

ReenigneArcher
ReenigneArcher previously approved these changes Mar 10, 2023
@ReenigneArcher

This comment was marked as resolved.

@meteiorman

This comment was marked as resolved.

@ReenigneArcher

This comment was marked as resolved.

@meteiorman

This comment was marked as resolved.

@ReenigneArcher
Copy link
Member

I pushed a commit that allows us to get any asset from the /web directory. Still seeing some issues.

  • Changing theme only seems to update the navbar color.
    • Dark theme has black/dark gray navbar, text stays dark so can't read text.
    • Light theme looks like before in navbar.

I didn't look much into the reason, but didn't see any errors in console.

@ReenigneArcher ReenigneArcher mentioned this pull request Mar 18, 2023
11 tasks
@DanTheMan827
Copy link

A JavaScript toggle works, but most websites just use css media queries for dark/light mode if they have separate styles

@meteiorman
Copy link
Author

meteiorman commented Mar 20, 2023

A JavaScript toggle works, but most websites just use css media queries for dark/light mode if they have separate styles

Since he's running Bootstrap on the page I went ahead and tapped into the bootstrap functionality which uses a class on the page. I'm not able to troubleshoot the issue @ReenigneArcher is having yet, though if you have any ides @DanTheMan827

This is the Codepen I built to kick things off, if it helps: https://codepen.io/jspeer/pen/KKxQWBy

@ReenigneArcher
Copy link
Member

ReenigneArcher commented Mar 26, 2023

Here are some screenshots.

Auto:
image

Light:
image

Dark:
image

Some of the js/css is slow to load, but every resource does load after 1 or 2 seconds at least.

The head tag does have the data-bs-theme attribute changed. Looking at the docs (https://getbootstrap.com/docs/5.3/customize/color-modes/) it appears this feature was just added in v5.3. Your branch is using v5.2.3. I'm not sure why @dependabot hasn't tried to update this dependency yet, but I believe that is likely the reason it's not fully working.

Edit: latest bootstrap on npm is v5.2.3.
image

@ReenigneArcher ReenigneArcher marked this pull request as draft March 28, 2023 01:07
@ReenigneArcher
Copy link
Member

Updating w/ rebase to make this easier to fix conflicts after adjusting clang-format rules

@ReenigneArcher ReenigneArcher force-pushed the feature/web-darktheme branch from c5c0d01 to ffcd138 Compare March 28, 2023 01:17
@meteiorman
Copy link
Author

Good catch, that's totally the issue - I'm basing it off of 5.3 - since you're using stock Bootstrap, I had pulled the latest from their page to get the color picker to work. I wasn't sure how those packages get updated so I didn't commit that latest version in my code. You could pause this until 5.3 is out of alpha, or go with alpha 3 for now and it'd probably work just fine as well.

@ReenigneArcher
Copy link
Member

@meteiorman bootstrap has been updated to v5.3.0 now. Could you fix the conflicts in your PR and we can resume this development/testing?

@ReenigneArcher
Copy link
Member

Would like to have this feature added, but closing due to no response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants