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

Allow the WebShell dark mode toggle to be disabled #305

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

notthetup
Copy link
Collaborator

@notthetup notthetup commented Feb 20, 2024

We added a toggle button for Light/Dark mode on the fjåge Web Shell. However, many modern web frameworks support dark mode, and it's common to embed the WebShell as an iframe inside web apps made from such a framework.

So we add two features to WebShell.

  • WebShell now looks at the class attribute on the parent HTML element and if it finds a light or dark class attribute uses that as a hint to hide its toggle button and use the parent element's mode (light/dark) as it's own mode.
  • WebShell also looks at an attribute on the iframe called hideModeToggle If that's set to true, then it also hides its toggle button.
  • The WebShell accepts a message sent using the window.postMessage API with a theme property which defines the theme to be either light or dark. This allows the container web app to update the WebShell that the theme has changed.

Finally, we updated the button style for the toggle button.

@notthetup notthetup force-pushed the fjage-webshell-darkmode-disable branch from d911ec5 to 745ecfa Compare February 22, 2024 03:03
@notthetup notthetup requested a review from ettersi February 27, 2024 06:53
@notthetup notthetup self-assigned this Feb 27, 2024
Copy link
Collaborator

@ettersi ettersi left a comment

Choose a reason for hiding this comment

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

LGTM other than a few tiny nitpicks 👍


  • WebShell now looks at the class attribute on the parent HTML element and if it finds a light or dark class attribute uses that as a hint to hide its toggle button and use the parent element's mode (light/dark) as it's own mode.

Technically this could conflict with another component which uses toplevel light / dark class attributes for some other purpose. Super unlikely, of course, and probably not worth changing, but it does trigger some voices in my brain...

src/main/resources/org/arl/fjage/web/shell/index.html Outdated Show resolved Hide resolved
src/main/resources/org/arl/fjage/web/shell/index.html Outdated Show resolved Hide resolved
@notthetup
Copy link
Collaborator Author

Technically this could conflict with another component which uses toplevel light / dark class attributes for some other purpose. Super unlikely, of course, and probably not worth changing, but it does trigger some voices in my brain...

Indeed, there is no other way to do this other than assume that the parent will send the postMessage at initialization. There are lots of issues there as well with things being loaded asynchronously.

On top of that adding a class to the html is pretty rare and I've only seen frameworks do it for this purpose. So I think we should be safe for now.

@notthetup notthetup merged commit 24e52cc into master Feb 27, 2024
1 of 2 checks passed
@notthetup notthetup deleted the fjage-webshell-darkmode-disable branch February 27, 2024 09:16
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