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

Feature suggestion auto/default theme #13

Closed
ili101 opened this issue Jan 8, 2021 · 27 comments · Fixed by #14
Closed

Feature suggestion auto/default theme #13

ili101 opened this issue Jan 8, 2021 · 27 comments · Fixed by #14
Assignees
Labels
enhancement ⬆️ New feature or request
Milestone

Comments

@ili101
Copy link
Contributor

ili101 commented Jan 8, 2021

Use-PodeWebTemplates -Title Tools -Theme Auto
https://css-tricks.com/a-complete-guide-to-dark-mode-on-the-web/
image

@Badgerati
Copy link
Owner

This is a good idea! I've been meaning to go back an tweak the themes a bit so Light/Dark could be toggled from the frontend.

My thinking is, like you've put above, we have a new Auto which is the default theme; this auto-selects Light or Dark depending on the user's system settings. If either Auto/Light/Dark are the chosen theme, then a toggle is displayed on the pages - in the header nav, so the user can switch at will.

If another theme, like Terminal, or any others that may get added, are chosen, the toggler isn't displayed.

Or, if we have a Light/Dark pair - so Light/Dark, TerminalLight/TerminalDark, etc - we could do something like:

# default
Use-PodeWebTemplates -Title Tools -Theme Default -ThemeStyle Dark
Use-PodeWebTemplates -Title Tools -Theme Default -ThemeStyle Light
Use-PodeWebTemplates -Title Tools -Theme Default -ThemeStyle Auto

# terminal
Use-PodeWebTemplates -Title Tools -Theme Terminal -ThemeStyle Dark
Use-PodeWebTemplates -Title Tools -Theme Terminal -ThemeStyle Light
Use-PodeWebTemplates -Title Tools -Theme Terminal -ThemeStyle Auto

that could be interesting 🤔

@ili101
Copy link
Contributor Author

ili101 commented Jan 9, 2021

I was thinking of implementing something like this by myself.
For example I will create a tool with form login.
I create a user config page where the user sets its preferences for the tool and also the theme override for example.

I don't know if this is something to include built into the framework or something to leave for the developer to set up and provide an example of that can be copied and customized.

I don't understated yet how the session information is saved in Pode. for example in the tool I created if I login with a user, then stop and start the server and refresh the page I get logged out. why is that?
If the session login is saved in the cookie by default as stated here https://badgerati.github.io/Pode/Functions/Middleware/Enable-PodeSessionMiddleware/#cookies-default then why after the server is restarted it's not persists with the cookie?
Is the login information is not in the cookie but in the server memory? (So I need to use -storage for the login and the theme selection for example to persist restart?)

@Badgerati
Copy link
Owner

The session cookie is purely the current session's ID, it can be a cookie or a header. The session data itself is stored within Pode's default in-memory session storage, and when the server is restarted this storage is wiped. Each request from the client is sent to Pode server with the session cookie, that sessionId is verified, and if it doesn't exist in the storage then a 401 is returned (hence logging out after a restart).

This is handy for simple setups where no data is ever needed, but if session data should persist a custom -Storage is needed - this can be file/database/document/etc based. However, I wouldn't store user/theme data in sessions - because the moment the session expires, BAM, all gone! 👀 (unless the session lives for like 100yrs of course, haha 😂)

User config should be stored where ever your users' details are stored, that way it persists through sessions. So if it was some AD, that could be Name, DOB, Department, etc.

For something like the theme, obviously AD doesn't have anything for that. Normally something like this would be stored as a separate cookie, say pode.web.theme. If the cookie is set you can use the theme defined, if not then use the default theme. Even if the session expires, that cookie survives - so long as it doesn't have an expiry. You can get the cookie in Pode via Get-PodeCookie or in JavaScript via document.cookie.

I do want to simplify the way the themes get set, something like <body pode-theme="light">, so switching it is easier.

It would also be nice if Pode.Web has internal support to check for a pode.web.theme cookie, and auto-switch themes for you - it's still controlled by yourself, and you have to make a custom settings/theme-picker page, but make it simple so you only have to set the pode.web.cookie. Could even have a setPodeTheme(...) in Pode.Web's JavaScript you could call and it deals with it.

@ili101
Copy link
Contributor Author

ili101 commented Jan 10, 2021

I played with this today to understand how it works, here what I came up with ili101/PWT@3973c90 (rough concept 😬)

  1. Created a PodeAuthUserFile with 2 users for testing.
  2. Added Middleware Storage with SQLite to save the login sessions.
  3. Added a Config page that shows the theme and button that flips it.
    image

There are 2 things I didn't manage to do:
How to refresh the page after clicking the flip theme button?
The theme is flipped for everyone. How do I flip it only in the scope of this user?

@ili101
Copy link
Contributor Author

ili101 commented Jan 11, 2021

Added editing your user configs from DB
ili101/PWT@4c476fd
image

@Badgerati
Copy link
Owner

@ili101 Whoa, very cool!

The theme gets changed for everyone, as Set-PodeState updates the state of the server as a whole. This is where I was saying, like there's the pode.web.theme state value, there could also be a pode.web.theme cookie - and Pode.Web checks for this internally, as this would be to a user's scope.

Another idea could be to have a -ThemeProperty on Set-PodeWebLoginPage. This would be the property Pode.Web checks for against an authenticated user, and uses that theme over the default. This would be easier if you're storing the theme for a user in a data store somewhere.

Actually, both would be neat: the cookie for people who are using an auth like AD and the theme is cookie based/cached, and the property for people who have a custom store and can store the theme for a user 🤔

As for refreshing the page: at the moment, not possible, but it's a simple Out-PodeWebRefreshPage that can be added!

@ili101
Copy link
Contributor Author

ili101 commented Jan 11, 2021

Interesting. So in the example I made if I use AD authentication and I have the SQLite with all the user configuration including the theme.
I will need to provide a scriptblock that retrieves the user theme somewhere or set some state variable/cookie with the theme from the SQL no?

@Badgerati
Copy link
Owner

With the inbuilt Windows AD, the user object won't have anything like a "theme" property. You could of course use an AD, and then store extra settings in some SQLite DB; using some middleware to get this extra config using the user's email for example.

So it would go:

  • Windows auth to get initial user object
  • Middleware to use $WebEvent.Auth.User.Email to query a database for additional config
  • Add the extra config to the user object, ie:
$mware = {
    # get the config for the user
    $config = Get-UserConfigFromSql -Email $WebEvent.Auth.User.Email

    # add theme prop
    $WebEvent.Auth.User.Theme = $config.Theme

    # or add all config:
    $WebEvent.Auth.User.CustomConfig = $config
}

Add-PodeRoute -Middleware $mware -Path etc, etc

You would need to set that middleware on each route, because it would need to route after auth. This would only apply to any inbuilt auth that gets the user object directly - such as Windows AD, File, and IIS. Raw basic, form, and even Azure AD you won't need an extra middleware as these are all piped into Add-PodeAuth.

If the theme was stored at say $WebEvent.Auth.User.Theme, and we added the logic, then Pode.Web would handle the theme and you'd only need to make sure the .Theme prop was set for a user, and do:

Set-PodeWebLoginPage -Authentication Example -ThemeProperty Theme
  • if no user: use default theme
  • if user, check .Theme prop, if present use that, else use default theme

@ili101
Copy link
Contributor Author

ili101 commented Jan 11, 2021

Ok I think I understated now 🤔
$WebEvent.Auth.User.Theme sounds good.
Regarding setting it. For all the methods that use Add-PodeAuth it's no problem as you can set it in the -ScriptBlock but in Add-PodeAuthIIS, Add-PodeAuthUserFile, Add-PodeAuthWindowsAd, Add-PodeAuthWindowsLocal you are limited.
So maybe to make them more consistent with Add-PodeAuth add -ScriptBlock to them to?
The ScriptBlock will run after the built-in one and allow you to add extra actions like setting $WebEvent.Auth.User.Theme, overriding user Groups or whatever else you need to add/edit/log/etc...
What do you think? probably more efficient/friendly then running a second middleware?

@Badgerati
Copy link
Owner

As I was writing that post yesterday I was also thinking the same thing! 😂 I'll see if it's feasible and write it up, adding an optional -ScriptBlock to those inbuilt methods.

@Badgerati
Copy link
Owner

In the above commit, I've rejigged the theming logic a bit. If you set a Theme property on your user objects then Pode.Web will auto-detect it, and use that theme instead of the server default.

@ili101
Copy link
Contributor Author

ili101 commented Jan 13, 2021

In the above commit, I've rejigged the theming logic a bit. If you set a Theme property on your user objects then Pode.Web will auto-detect it, and use that theme instead of the server default.

Update theme from DB from 😜
PodeWeb

Badgerati added a commit that referenced this issue Jan 13, 2021
@Badgerati
Copy link
Owner

Awesome!

and rats! I knew I forgot to add something: added a new Reset-PodeWebPage function now, so you can use on that form submit to refresh the page 😄

@ili101
Copy link
Contributor Author

ili101 commented Jan 14, 2021

My pilot project is in mostly usable state 😀 all the major things are working 🙏
Those are the Pode related comments left in my project.

  • TODO: Pode.Web: -Theme Auto?
  • TODO: Pode.Web: Remove 'Choose an option'?
    Probably can remove this code so user can set his own first option (like Auto, Default, etc)
            if (!$data.Multiple) {
                if ([string]::IsNullOrWhiteSpace($data.SelectedValue)) {
                    "<option selected>Choose an option</option>"
                }
                else {
                    "<option>Choose an option</option>"
                }
            }
  • TODO: Pode: 'Enable-PodeErrorLogging -Levels' works?
    It catches terminating errors, didn't manage to make it log something else like warning etc. Is it suppose to?
  • TODO: Pode: -Extend not implemented in Pode (with or without -Storage)?
    I don't think it's doing something. (Not really planing to use it, just stumbled upon it while testing)

@Badgerati
Copy link
Owner

  • -Theme Auto I will look at, it shouldn't be all that hard with the new theme cookie support
  • What about a new -NoChooseOption switch?
  • Do you have an example of it not working? Anything from within Pode is all Error level, nothing less, if that's what you mean? If you're using a custom Write-PodeErrorLog with -Level Warning and Enable-PodeErrorLogging is set to allow warnings, then yeah, that should be working.
  • You're not going crazy, I spotted it too! Fixed in Sessions don't get extended internally from AJAX requests Pode#652 for Pode 2.1.0 😄

@ili101
Copy link
Contributor Author

ili101 commented Jan 16, 2021

  • 👍
  • I don't have a strong preference regarding -NoChooseOption. The use cases are probably: 1. have a default option, 2. need a different first text, 3. ok with "Choose an option". Whatever you think is best.
  • I see, so yes it's working as you describe. I first thought it will capture Write-Warning. Maybe it will be good to add to https://badgerati.github.io/Pode/Tutorials/Logging/Types/Errors/ something like: Terminating errors are automatically captured, other error levels need to be sent with Write-PodeErrorLog
  • Tested looks good with and without -Storage 🎉

@ili101
Copy link
Contributor Author

ili101 commented Jan 17, 2021

Is it finished or partial commit? with 2b7d28b I always get dark

Import-Module -Name '\Pode\src\Pode.psd1', '\Pode.Web\src\Pode.Web.psd1' -Force
Start-PodeServer {
    Add-PodeEndpoint -Address localhost -Protocol Http
    Use-PodeWebTemplates -Title 'Example' -Theme Auto
    Set-PodeWebHomePage -Title 'T'
}

Think I found it

var isSystemDark = window.matchMedia('(prefers-color-scheme: dark)');

need to have "matches"
var isSystemDark = window.matchMedia('(prefers-color-scheme: dark)').matches;

Also once cookie is set it's there forever (In case Windows theme setting is changed)? I tried to make a reset button but I think Remove-PodeCookie is not forking from Pode.Web (works from Add-PodeRoute directly)

Start-PodeServer {
    Add-PodeEndpoint -Address localhost -Protocol Http
    Use-PodeWebTemplates -Title 'Example' -Theme Auto

    Add-PodeWebPage -Name kk -ScriptBlock {
        New-PodeWebCard -Name 'Search' -Content @(
            New-PodeWebText -Value (Get-PodeCookie -Name 'pode.web.theme' | Out-String)
            New-PodeWebButton -Name 'Remove' -ScriptBlock {
                Remove-PodeCookie -Name 'pode.web.theme'
            }
        )
    }
    Add-PodeRoute -Method Get -Path '/remove' -ScriptBlock {
        Remove-PodeCookie -Name 'pode.web.theme'
    }
}

Badgerati added a commit that referenced this issue Jan 17, 2021
@Badgerati
Copy link
Owner

Yep, .matches was missing!

The cookie at the moment is set to expire after 30 days - normally I wouldn't expect people to flip from light/dark system themes very often, if at all. I could drop to 7 days, or make it so the cookie auto-expires when the browser is closed?

The Remove-PodeCookie does kinda work on the button, but seems AJAX is a little more strict - I've a fix I can do for that over in Pode.

An wow, I can't believe I missed off Write-PodeErrorLog on that page! 🙈

@ili101
Copy link
Contributor Author

ili101 commented Jan 17, 2021

I tried to understand how it works yesterday. I made the JS update the cookie every time and added an extra test in the PS code to use the cookie or ignore it. so it's updated on every page load, but then I can't know if I need to refresh the page from the JS or not so it can't refresh automatically to show the theme ili101@89c464f. Anyway leave it as is 30 days is good I think.

@Badgerati
Copy link
Owner

Aaah, you gave me an idea. That commit is a little cleverer on detecting system theme switches - and keeps in memory if the theme was Auto.

If auto, get system and body theme. If they match, do nothing, if not, update cookie and refresh.

@ili101
Copy link
Contributor Author

ili101 commented Jan 17, 2021

Aaah, you gave me an idea. That commit is a little cleverer on detecting system theme switches - and keeps in memory if the theme was Auto.

If auto, get system and body theme. If they match, do nothing, if not, update cookie and refresh.

It have similar problem to what I got yesterday, if there is an Auth Theme override that is different from the system theme it gets in infinite refresh loop, this is why in ili101@89c464f I put it in Get-PodeWebTheme after the Get-PodeWebAuthTheme test.
But after adding this test to the new parameter it looks like it works 🤞 https://github.com/Badgerati/Pode.Web/compare/Issue-13...ili101:ThemeAuto?expand=1

@Badgerati
Copy link
Owner

Another way could be to change the priorities when finding the theme. Instead of Auth>Cookie>Server, make it Cookie>Auth>Server 🤔

function Get-PodeWebTheme
{
    [CmdletBinding()]
    param()


    $theme = Get-PodeWebCookie -Name 'theme'
    if (($null -ne $theme) -and ![string]::IsNullOrWhiteSpace($theme.Value)) {
        return $theme.Value.ToLowerInvariant()
    }

    $theme = Get-PodeWebAuthTheme -AuthData (Get-PodeWebAuthData)
    if (![string]::IsNullOrWhiteSpace($theme)) {
        return $theme.ToLowerInvariant()
    }

    return (Get-PodeWebState -Name 'theme').ToLowerInvariant()
}

@ili101
Copy link
Contributor Author

ili101 commented Jan 19, 2021

But then Auth will never be applied as Cookie will be there.
However we can make it work by setting the Cookie to the theme we need, then it's ok to always use the cookie if it's there.
To do this I removed "pode-theme-auto" and added "pode-theme-target" and set cookie to that (if pode-theme-target is auto it sets cookie to system theme) https://github.com/Badgerati/Pode.Web/compare/Issue-13...ili101:ThemeAuto2?expand=1
This way default can be anything and it can be overridden by anything in the Auth

@Badgerati
Copy link
Owner

Implemented your idea of using target theme instead, seems like it should cover most use cases.

Only change I made was to add an -IgnoreCookie switch on Get-PodeWebTheme - just so index/login.pode are a little cleaner.

@ili101
Copy link
Contributor Author

ili101 commented Jan 19, 2021

Nice. that's cleaner 😀
Can you add the -NoChooseOption?
What do you think abut merging all the branches here and in Pode? Starts to get hard to track all the branches

@Badgerati
Copy link
Owner

-NoChooseOption added. If it's good, I'll get his all merged.

On Pode, you mean the ScriptBlock/auth ticket? Gonna do some last checks on it and merge later hopefully.

@ili101
Copy link
Contributor Author

ili101 commented Jan 19, 2021

Tested. all good in this issue.
Ahhh I see you already merged most of them 🙏, yes the last one I use is the ScriptBlock/auth

@Badgerati Badgerati added this to the 0.3.0 milestone Jan 19, 2021
@Badgerati Badgerati added the enhancement ⬆️ New feature or request label Jan 19, 2021
@Badgerati Badgerati self-assigned this Jan 19, 2021
@Badgerati Badgerati mentioned this issue Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ⬆️ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants