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

Dynamic Material Theme (Dark/Light) #1380

Merged
merged 24 commits into from
Aug 30, 2023
Merged

Dynamic Material Theme (Dark/Light) #1380

merged 24 commits into from
Aug 30, 2023

Conversation

marek-guran
Copy link
Contributor

@marek-guran marek-guran commented Aug 7, 2023

About

Material You Inspired dark and light theme, which supports both stable and insiders versions. It uses PHP to calculate colors from color picker in system tab.

⚠ Themes support only dark color pallete, light color pallete will make everything unreadable
❌ Wasnt able to make them switchable by settings

Screenshots - PC (light)

Snímka obrazovky (7)
Snímka obrazovky (8)
Snímka obrazovky (9)

Screenshots - PC (dark)

Snímka obrazovky (4)
Snímka obrazovky (5)
Snímka obrazovky (6)

Screenshots - Mobile (light)

Screenshot_20230807-184551_Chrome
Screenshot_20230807-184622_Chrome
Screenshot_20230807-184657_Chrome

Screenshots - Mobile (dark)

Screenshot_20230807-184853_Chrome
Screenshot_20230807-184912_Chrome
Screenshot_20230807-184935_Chrome

@marek-guran marek-guran marked this pull request as draft August 7, 2023 16:25
@marek-guran marek-guran marked this pull request as ready for review August 7, 2023 16:52
@marek-guran
Copy link
Contributor Author

Latest 2 commits fix these 2 buttons:

Screenshot_20230808-064159_Chrome~2
Screenshot_20230808-064141_Chrome~2

@marek-guran marek-guran changed the title Material theme Dynamic Material Theme (Dark/Light) Aug 8, 2023
@billz
Copy link
Member

billz commented Aug 9, 2023

Thanks for the PR! I'm traveling for the next few days - will give this a closer look when I return.

@billz
Copy link
Member

billz commented Aug 25, 2023

In the minimal testing I've done the 'theme' cookie value isn't being set for some reason.
This results in an 'undefined' stylesheet being loaded. Needs troubleshooting.

@billz
Copy link
Member

billz commented Aug 28, 2023

@marek-guran new themes also need to be defined here:

// Define themes
var themes = {
"default": "custom.php",
"hackernews" : "hackernews.css",
"lightsout" : "lightsout.css",
}

@marek-guran
Copy link
Contributor Author

@marek-guran new themes also need to be defined here:

// Define themes
var themes = {
"default": "custom.php",
"hackernews" : "hackernews.css",
"lightsout" : "lightsout.css",
}

Yes, I found out about this yesterday. For some reason it's not able to change to my themes, I am currently troubleshooting it and trying to make it work.

Still cant change themes in settings
@marek-guran marek-guran marked this pull request as draft August 28, 2023 12:32
@marek-guran
Copy link
Contributor Author

marek-guran commented Aug 28, 2023

Managed to make it work, for some reason it wasnt switching because of some caching so it did not get updated (even after today commit).

Then reloaded the page on one device (cleaned cache) and it switched themes correctly. But other device lets say phone that had it cached, refused to change it.

Screenshots:
Screenshot_20230828-151327_Chrome
Screenshot_20230828-151330_Chrome
Screenshot_20230828-151336_Chrome

@marek-guran marek-guran marked this pull request as ready for review August 28, 2023 13:03
@billz
Copy link
Member

billz commented Aug 28, 2023

Confirmed themes load from the menu now.

Untitled

I noticed that the card-body class has border-radius: 18px, which looks somewhat odd with the headers and footers. What do you think?

@marek-guran
Copy link
Contributor Author

marek-guran commented Aug 28, 2023

Confirmed themes load from the menu now.

Untitled

I noticed that the card-body class has border-radius: 18px, which looks somewhat odd with the headers and footers. What do you think?

Yes, the issue is that the whole layout of items placed are basically cards on cards and it makes that effect. Basically it could be hidden, but if I did it... I might break default themes. I will try to fix it and not break them. Will take a look on it and mark this as draft until the issue is not resolved and you don't get spammed by my commits.

I got an idea with the lazy way and adding in CSS for the header and footer the 18 px down and up so it could overlap the cards or make the footer and header to be floating on top of them and then giving them defined space which would hide the unwanted corners.

Removed dark theme since it will be enabled by switch at top bar
Updated dark theme switcher so now it can switch between dark and light material theme while keeping it's functionality to stock theme.
@marek-guran marek-guran marked this pull request as draft August 28, 2023 21:41
1. The rounded corners issue is gone
2. Fixed light theme status indicator, so on latest insiders it wont hide active green button
3. Added new visuals to navigation menu to make it more clear what card is active
@marek-guran
Copy link
Contributor Author

The issue is fixed and improved both themes. Now it is in settings as one theme and can be switchable by the dark theme switcher. Also fixed for insiders light theme (changed background color to secondary color) for status indicator button that switched from text to icon. Also the navigation menu got improved indicators for active items, so it is more clear to see what is active.

Material Theme (Non insiders look)

material-noninsiders-look.mp4

Material Theme (Insiders look)

material-insiders-look.mp4

@marek-guran marek-guran marked this pull request as ready for review August 29, 2023 09:09
@billz
Copy link
Member

billz commented Aug 29, 2023

Looking good! Seems like the card-body is missing some vertical padding. This is with padding: 2.25rem 1.25rem 2.25rem 1.25rem;

image

Just a suggestion.

The dark mode support works great 😎 We could probably just refer to the theme as 'Material' (exclude 'Use dark colors'). Users will figure this out pretty quick, I think.

@marek-guran
Copy link
Contributor Author

Looking good! Seems like the card-body is missing some vertical padding. This is with padding: 2.25rem 1.25rem 2.25rem 1.25rem;

image Just a suggestion.

The dark mode support works great 😎 We could probably just refer to the theme as 'Material' (exclude 'Use dark colors'). Users will figure this out pretty quick, I think.

I have fixed it the lazy way, so made the header go down by -18px (this hides the visual error - cant be fixed other way). Will take a look on this and see if I can fix it.

@billz
Copy link
Member

billz commented Aug 29, 2023

One minor annoyance is the custom WireGuard icon doesn't highlight on mouseover like the other nav-items.

image

This isn't an issue with your theme — it's always done this. I just haven't looked into it.
It's a nice-to-have, not critical to this PR.

@marek-guran
Copy link
Contributor Author

marek-guran commented Aug 29, 2023

One minor annoyance is the custom WireGuard icon doesn't highlight on mouseover like the other nav-items.

image This isn't an issue with your theme — it's always done this. I just haven't looked into it. It's a nice-to-have, not critical to this PR.

I tried but wasnt able to change it, there were some issues with type of file i think? It did not want to act like normal icons from fa fa class. Also the padding is now fixed and renamed it as you suggested. (See below images - its for both dark and light theme, just did not want to spam with screenshots)
Snímka obrazovky (19)
Snímka obrazovky (20)

@billz
Copy link
Member

billz commented Aug 29, 2023

Nice work. You can also apply your theme colors to the link quality graph, if you wish.

// Support for dark theme
theme = getCookie('theme');
if (theme == 'lightsout.css') {
var bgColor1 = '#141414';

I've considered changing the backgroundColor property to 'transparent' since I think this looks better with all themes.

@marek-guran
Copy link
Contributor Author

Nice work. You can also apply your theme colors to the link quality graph, if you wish.

// Support for dark theme
theme = getCookie('theme');
if (theme == 'lightsout.css') {
var bgColor1 = '#141414';

I've considered changing the backgroundColor property to 'transparent' since I think this looks better with all themes.

Sure, will take a look on it.

@marek-guran
Copy link
Contributor Author

marek-guran commented Aug 29, 2023

So I tried to implement it, I tried many ways but made it furthest with adding this to theme:

// Save colors to a text file
$colors = array(
    'textColor' => $textColor,
    'cardsColor' => $cardsColor,
    'secondaryColor' => $secondaryColor,
    'primaryColor' => $primaryColor,
    'backgroundColor' => $backgroundColor
);

$file = fopen('colors.json', 'w');
fwrite($file, json_encode($colors));
fclose($file);

This basically saves the colors to .json which looks for example like this:

{"textColor":"#fdf4f6","cardsColor":"#ec91a6","secondaryColor":"#e46482","primaryColor":"#d8224c","backgroundColor":"#efa7b7"}

Then I was trying to load them by that JavaScript code with:

// Function to fetch JSON data
function fetchColors() {
  return fetch('/app/css/colors.json')
    .then(response => response.json())
    .catch(error => {
      console.error('Error loading colors:', error);
    });
}

// Fetch colors from the JSON file
var colorsPromise = fetchColors();

// Support for dark theme
theme = getCookie('theme');
if (theme == 'lightsout.css') {
  var bgColor1 = '#141414';
  var bgColor2 = '#141414';
  var borderColor = 'rgba(37, 153, 63, 1)';
  var labelColor = 'rgba(37, 153, 63, 1)';
} else if (theme == 'material-light.php') {
  // Load colors from the fetched data
  colorsPromise.then(colors => {
    var bgColor1 = "'"+colors.primaryColor+"'";
    var bgColor2 = "'"+colors.primaryColor+"'";
    var borderColor = "'"+colors.secondaryColor+"'";
    var labelColor = "'"+colors.textColor+"'";

    // Print colors to the console
    console.log("bgColor1:", bgColor1);
    console.log("bgColor2:", bgColor2);
    console.log("borderColor:", borderColor);
    console.log("labelColor:", labelColor);
  });
} else {
  var bgColor1 = '#d4edda';
  var bgColor2 = '#eaecf4';
  var borderColor = 'rgba(147, 210, 162, 1)';
  var labelColor = 'rgba(130, 130, 130, 1)';
}
Rest of the code...

But no matter what I tried... For some reason it loaded full black or white colors for link quality. Most likely will have to use hard coded values to make it readable on all color schemes. But will try to make it work. If you have any suggestions or see any errors in my way how I want to achieve these colors, please let me know.

];
$selectedTheme = array_search($_COOKIE['theme'], $themeFiles);

// widget options
$widgetOpt = $_SESSION["widgetOpt"];
Copy link
Member

Choose a reason for hiding this comment

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

$widgetOpt is from the Insiders edition 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$widgetOpt is from the Insiders edition 😉

Sorry my bad, didnt notice it here when i was copying the files to github. Now it is fixed and also made it the easy way and hard coded the color values. Now it uses white with transparency also. Chose this variant since it looked to me like best for both versions.

Snímka obrazovky (21)
Snímka obrazovky (22)

@billz
Copy link
Member

billz commented Aug 30, 2023

No problem, for this PR it's not an issue. Post-merge I'll set the background color to transparent.

Added hard coded colors to material theme. Now it is more readable on dark and light versions.
@billz
Copy link
Member

billz commented Aug 30, 2023

@marek-guran well done! we're good to merge. this is a great contribution to the project

@billz billz merged commit 16a413a into RaspAP:master Aug 30, 2023
3 checks passed
@billz
Copy link
Member

billz commented Sep 13, 2023

@marek-guran looks like the modal dialog could use some attention.

modal

Not super critical, so rather than create a new issue I thought I'd mention it here.

@marek-guran
Copy link
Contributor Author

@marek-guran looks like the modal dialog could use some attention.

modal Not super critical, so rather than create a new issue I thought I'd mention it here.

Oh... Will fix this once I find time for it.

@marek-guran
Copy link
Contributor Author

Hey, just commenting to let you know that I didn't forget about this issue. I just didn't find time to fix this yet. It is still on my mind in To Do things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants