From 82e45dcc991a86f6889bc6bbf83143a46c6b6323 Mon Sep 17 00:00:00 2001
From: Iisakki Rotko <iisakki.rotko@gmail.com>
Date: Fri, 23 Feb 2024 15:42:52 +0100
Subject: [PATCH] incorporate suggestions from code review

Co-authored-by: Maarten Breddels <maartenbreddels@gmail.com>
---
 solara/__main__.py                     | 21 ++++++++++++------
 solara/lab/components/theming.py       | 10 ++++-----
 solara/lab/components/theming.vue      |  2 +-
 solara/server/server.py                |  4 +++-
 solara/server/settings.py              |  1 -
 solara/server/templates/solara.html.j2 |  6 ++----
 solara/tasks.py                        | 30 +++++++++++++++++---------
 solara/website/pages/api/theming.py    | 13 ++++++-----
 8 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/solara/__main__.py b/solara/__main__.py
index cea8074f3..172f6d47e 100644
--- a/solara/__main__.py
+++ b/solara/__main__.py
@@ -11,12 +11,13 @@
 
 import rich
 import rich_click as click
-import solara
 import uvicorn
 from rich import print as rprint
-from solara.server import settings
 from uvicorn.main import LEVEL_CHOICES, LOOP_CHOICES
 
+import solara
+from solara.server import settings
+
 from .server import telemetry
 
 try:
@@ -216,11 +217,17 @@ def cli():
     default=settings.theme.variant.name,
     help=f"Use light or dark variant, or auto detect (auto). [default: {settings.theme.variant.name}",
 )
+@click.option(
+    "--dark",
+    type=bool,
+    default=settings.theme.variant == settings.ThemeVariant.dark,
+    help="Use dark theme. Shorthand for --theme-variant=dark",
+)
 @click.option(
     "--theme-variant-user-selectable/--no-theme-variant-user-selectable",
     type=bool,
-    default=settings.theme.variant_user_selectable,
-    help=f"Can the user select the theme variant from the UI. [default: {settings.theme.variant_user_selectable}",
+    hidden=True,
+    help="Deprecated.",
 )
 @click.option("--pdb/--no-pdb", "use_pdb", default=False, help="Enter debugger on error")
 @click.argument("app")
@@ -273,6 +280,7 @@ def run(
     use_pdb: bool,
     theme_loader: str,
     theme_variant: settings.ThemeVariant,
+    dark: bool,
     theme_variant_user_selectable: bool,
     ssg: bool,
     search: bool,
@@ -375,12 +383,13 @@ def open_browser():
     kwargs["loop"] = loop
     settings.main.use_pdb = use_pdb
     settings.theme.loader = theme_loader
+    if dark:
+        theme_variant = settings.ThemeVariant.dark
     settings.theme.variant = theme_variant
-    settings.theme.variant_user_selectable = theme_variant_user_selectable
     settings.main.tracer = tracer
     settings.main.timing = timing
     items = (
-        "theme_variant_user_selectable theme_variant theme_loader use_pdb server open_browser open url failed dev tracer"
+        "theme_variant_user_selectable dark theme_variant theme_loader use_pdb server open_browser open url failed dev tracer"
         " timing ssg search check_version production".split()
     )
     for item in items:
diff --git a/solara/lab/components/theming.py b/solara/lab/components/theming.py
index cf509819c..22c1208a0 100644
--- a/solara/lab/components/theming.py
+++ b/solara/lab/components/theming.py
@@ -4,12 +4,11 @@
 from ipyvuetify.Themes import Theme
 
 import solara
-import solara.server.settings as settings
 from solara.components.component_vue import component_vue
 from solara.tasks import Proxy
 
 theme = Proxy(Theme)
-ipyvuetify.Themes.theme = theme
+ipyvuetify.Themes.theme = theme  # type: ignore
 
 
 @component_vue("theming.vue")
@@ -30,6 +29,7 @@ def ThemeToggle(
     on_icon: str = "mdi-weather-night",
     off_icon: str = "mdi-weather-sunny",
     auto_icon: str = "mdi-brightness-auto",
+    enable_auto: bool = True,
 ):
     """
     Insert a toggle switch for user to switch between light and dark themes.
@@ -45,8 +45,8 @@ def Page():
     ## Arguments
     - `on_icon`: The icon to display when the dark theme is enabled.
     - `off_icon`: The icon to display when the dark theme is disabled.
-    - `auto_icon`: The icon to display when the theme is set to auto
-        (**note**: auto mode is only available if the server settings `theme.variant_user_selectable` is enabled).
+    - `auto_icon`: The icon to display when the theme is set to auto. Only visible if `enable_auto` is `True`.
+    - `enable_auto`: Whether to enable the auto detection of dark mode.
     """
 
     def sync_themes(selected_theme: str):
@@ -55,7 +55,7 @@ def sync_themes(selected_theme: str):
     return _ThemeToggle(
         theme_dark=theme.dark,
         event_sync_themes=sync_themes,
-        enable_auto=settings.theme.variant_user_selectable,
+        enable_auto=enable_auto,
         on_icon="mdi-weather-night",
         off_icon="mdi-weather-sunny",
         auto_icon="mdi-brightness-auto",
diff --git a/solara/lab/components/theming.vue b/solara/lab/components/theming.vue
index c29abdf31..c33f79395 100644
--- a/solara/lab/components/theming.vue
+++ b/solara/lab/components/theming.vue
@@ -12,7 +12,7 @@
 <script>
 module.exports = {
     mounted() {
-        if (window.solara && theme.variant_user_selectable) {
+        if (window.solara) {
             if (localStorage.getItem(':solara:theme.variant')) {
                 this.theme_dark = this.initTheme();
             }
diff --git a/solara/server/server.py b/solara/server/server.py
index 3f91947c1..64da8a532 100644
--- a/solara/server/server.py
+++ b/solara/server/server.py
@@ -12,6 +12,7 @@
 import ipywidgets
 import jinja2
 import requests
+
 import solara
 import solara.routing
 import solara.settings
@@ -293,7 +294,8 @@ def include_css(path: str) -> Markup:
         url = f"{root_path}{path}?v={hash}"
         # when < 10k we embed, also when we use a url, it can be relative, which can break the url
         embed = len(content) < 1024 * 10 and b"url" not in content
-        # Include the jupyterlab theme css directly, so we can change it on demand
+        # Always embed the jupyterlab theme CSS to make theme switching possible (see solara.html.j2 template)
+        # TODO: Prevent browser from caching the theme CSS files
         if path.endswith("theme-dark.css") or path.endswith("theme-light.css"):
             content_utf8 = content.decode("utf-8")
             code = content_utf8
diff --git a/solara/server/settings.py b/solara/server/settings.py
index e1c7cf634..e5f68558a 100644
--- a/solara/server/settings.py
+++ b/solara/server/settings.py
@@ -32,7 +32,6 @@ class ThemeVariant(str, Enum):
 
 class ThemeSettings(BaseSettings):
     variant: ThemeVariant = ThemeVariant.light
-    variant_user_selectable: bool = True
     loader: str = "solara"
 
     class Config:
diff --git a/solara/server/templates/solara.html.j2 b/solara/server/templates/solara.html.j2
index f8c6b8f9b..3538c3a1b 100644
--- a/solara/server/templates/solara.html.j2
+++ b/solara/server/templates/solara.html.j2
@@ -209,10 +209,8 @@
         const themeVariants = ['light', 'dark', 'auto']
         solara.preRendered = {{ pre_rendered_html | safe | length | tojson }} > 0
         var theme = {{ theme | tojson | safe }}
-        if (theme.variant_user_selectable) {
-            if (localStorage.getItem(':solara:theme.variant')) {
-                theme.variant = JSON.parse(localStorage.getItem(':solara:theme.variant'))
-            }
+        if (localStorage.getItem(':solara:theme.variant')) {
+            theme.variant = JSON.parse(localStorage.getItem(':solara:theme.variant'))
         }
         function prefersDarkScheme() {
             return window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches
diff --git a/solara/tasks.py b/solara/tasks.py
index 87ca735e5..6a8c8616a 100644
--- a/solara/tasks.py
+++ b/solara/tasks.py
@@ -151,16 +151,20 @@ def exception(self) -> Optional[Exception]:
         return self._exception.value
 
     @abc.abstractmethod
-    def retry(self) -> None: ...
+    def retry(self) -> None:
+        ...
 
     @abc.abstractmethod
-    def cancel(self) -> None: ...
+    def cancel(self) -> None:
+        ...
 
     @abc.abstractmethod
-    def __call__(self, *args: P.args, **kwargs: P.kwargs) -> None: ...
+    def __call__(self, *args: P.args, **kwargs: P.kwargs) -> None:
+        ...
 
     @abc.abstractmethod
-    def is_current(self) -> bool: ...
+    def is_current(self) -> bool:
+        ...
 
     def _prestart(self):
         self._result.value = TaskResult[R](latest=self._last_value, _state=TaskState.STARTING)
@@ -438,7 +442,8 @@ def task(
     f: None = None,
     *,
     prefer_threaded: bool = ...,
-) -> Callable[[Callable[P, R]], Task[P, R]]: ...
+) -> Callable[[Callable[P, R]], Task[P, R]]:
+    ...
 
 
 @overload
@@ -446,7 +451,8 @@ def task(
     f: Callable[P, Union[Coroutine[Any, Any, R], R]],
     *,
     prefer_threaded: bool = ...,
-) -> Task[P, R]: ...
+) -> Task[P, R]:
+    ...
 
 
 def task(
@@ -693,7 +699,8 @@ def use_task(
     dependencies: None = ...,
     raise_error=...,
     prefer_threaded=...,
-) -> Callable[[Callable[P, R]], Task[P, R]]: ...
+) -> Callable[[Callable[P, R]], Task[P, R]]:
+    ...
 
 
 @overload
@@ -703,7 +710,8 @@ def use_task(
     dependencies: None = ...,
     raise_error=...,
     prefer_threaded=...,
-) -> Task[P, R]: ...
+) -> Task[P, R]:
+    ...
 
 
 @overload
@@ -713,7 +721,8 @@ def use_task(
     dependencies: List = ...,
     raise_error=...,
     prefer_threaded=...,
-) -> Callable[[Callable[[], R]], "Task[[], R]"]: ...
+) -> Callable[[Callable[[], R]], "Task[[], R]"]:
+    ...
 
 
 @overload
@@ -723,7 +732,8 @@ def use_task(
     dependencies: List = ...,
     raise_error=...,
     prefer_threaded=...,
-) -> "Task[[], R]": ...
+) -> "Task[[], R]":
+    ...
 
 
 def use_task(
diff --git a/solara/website/pages/api/theming.py b/solara/website/pages/api/theming.py
index 5df6ff851..b1ae8d7b2 100644
--- a/solara/website/pages/api/theming.py
+++ b/solara/website/pages/api/theming.py
@@ -3,8 +3,8 @@
 
 Theming is provided to Solara through the [`ipyvuetify`](https://solara.dev/docs/understanding/ipyvuetify) package.
 Two themes are provided by default: light and dark.
-Control over the theme variant can be provided to the user through the `ThemeToggle` component, or at a lower level through the
-`solara.lab.theme` object.
+Control over the theme variant (light, dark, or auto) can be provided to the user through the `ThemeToggle` component. Developers can
+control theming through the `solara.lab.theme` object.
 
 ## Themes
 
@@ -19,7 +19,10 @@
 Dark theme can be enabled/disabled through `solara.lab.theme.dark = True` / `False` / `None`. When set to `None`, the theme is set to auto,
 which will follow the system theme if the user's browser supports detecting it.
 
-When the theme is set to auto, the active theme can be detected through `solara.lab.theme.dark_effective`, which is set to `True` or `False` depending on
+When using solara server, the default theme variant can be set in the server settings through the command line interface. See
+[here](https://solara.dev/docs/understanding/solara-server) for more information.
+
+The active theme can be detected through `solara.lab.theme.dark_effective`, which is set to `True` or `False` depending on
 whether dark theme is enabled or not.
 
 ## Example
@@ -30,7 +33,7 @@
 from solara.lab import theme as theme
 
 
-def color(colors):
+def change_color(colors):
     if "purple" in colors:
         theme.themes.light.info = "#8617c2"
     else:
@@ -47,7 +50,7 @@ def Page():
     solara.Info("Info message")
     solara.Error("Error message")
 
-    with solara.ToggleButtonsMultiple(on_value=color):
+    with solara.ToggleButtonsMultiple(on_value=change_color):
         solara.Button("Change Info", value="purple")
         solara.Button("Change Error", value="green")
 ```