feat: sync dashboard theme to plugin pages#8390
Conversation
…ontext Add theme query parameter propagation through the plugin page asset pipeline so that the bridge SDK initial context includes isDark.
Set data-theme on <html> and add color-scheme meta tag server-side to prevent flash when entering plugin pages in dark mode.
Append theme query parameter to iframe src URL and include isDark in the postMessage context. Watch uiTheme changes to re-send context.
Set data-theme attribute on document.documentElement when context includes isDark, enabling live theme switching via postMessage.
Add theme adaptation section to existing plugin-pages docs in both Chinese and English, covering CSS variables and onContext() usage.
Verify isDark in bridge SDK initial context with various theme params, and verify data-theme and color-scheme injection in rewritten HTML.
Replace string match for <head> with case-insensitive regex to handle uppercase tags and tags with attributes, preventing duplicate injection of color-scheme meta tag.
Replace hardcoded PurpleThemeDark check with suffix-based detection so all dark theme variants are recognized automatically.
Extract _get_request_theme and _apply_theme_to_html to eliminate duplicate theme-parsing logic and isolate HTML mutation. Use case- insensitive regex for head tag detection to prevent duplicate injection when tags use mixed casing.
Wrap long function call lines for consistency with project style.
There was a problem hiding this comment.
Code Review
This pull request introduces light/dark theme synchronization for Plugin Pages, automatically applying the theme state to the document element and propagating it via query parameters and the bridge SDK. The feedback highlights a few critical issues: potential HTML corruption in _apply_theme_to_html if the <html> tag already has a theme or if <head> is missing, a potential TypeError in the customizer store if uiTheme is undefined, and the loss of hash fragments when constructing the iframe source URL, which could break hash-based routing.
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_apply_theme_to_html, the fallback branch that injects<head>uses a case-sensitivehtml.replace("<html", ...)and produces<html><head>...without ensuring a closing</head>or preserving existing attributes, which could lead to malformed or brittle HTML; consider using a single regex-based insertion (withIGNORECASE) that safely augments an existing<head>or inserts a full<head>...</head>block. - In
PluginPagePage.vue, the theme string ('dark' | 'light') is derived inline fromcustomizer.isDarkwhen building the iframe URL whilesendIframeContextsendsisDarkseparately; to avoid future drift, consider centralizing theisDark→ theme string mapping in a small helper or computed and reuse it for both the iframe URL and context.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_apply_theme_to_html`, the fallback branch that injects `<head>` uses a case-sensitive `html.replace("<html", ...)` and produces `<html><head>...` without ensuring a closing `</head>` or preserving existing attributes, which could lead to malformed or brittle HTML; consider using a single regex-based insertion (with `IGNORECASE`) that safely augments an existing `<head>` or inserts a full `<head>...</head>` block.
- In `PluginPagePage.vue`, the theme string (`'dark' | 'light'`) is derived inline from `customizer.isDark` when building the iframe URL while `sendIframeContext` sends `isDark` separately; to avoid future drift, consider centralizing the `isDark` → theme string mapping in a small helper or computed and reuse it for both the iframe URL and context.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="252-253" />
<code_context>
+ html = html.replace(
+ head_match.group(0), f"{head_match.group(0)}{meta_tag}", 1
+ )
+ else:
+ html = html.replace("<html", f"<html><head>{meta_tag}</head>", 1)
+ return html
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The `<html` replacement path can generate invalid markup by breaking the opening `<html>` tag and discarding its attributes.
In the `else` branch of `_apply_theme_to_html`, `html.replace("<html", f"<html><head>{meta_tag}</head>", 1)` runs before the original `<html...>` tag is closed, which can produce markup like `<html><head>…</head> lang="en">` and drop all attributes from the `<html>` element. Consider either mirroring the `re.sub` approach above (match the full `<html…>` start tag and insert `<head>…</head>` after its closing `>`), or explicitly locate the end of the `<html>` tag and inject `<head>…</head>` immediately after it without modifying the original tag text.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…n HTML rewrite Strip any existing data-theme attribute before adding the new one to prevent duplicate attributes. Use case-insensitive regex for the <head> fallback insertion to avoid corrupting <html> tag attributes.
Guard against undefined uiTheme to prevent TypeError when the theme config has not been initialized.
… URL Extract themeParam computed to avoid drift between URL and context. Include hash fragment in iframe URL to support SPA hash routing.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
isDarkgetter incustomizer.tsrelies onuiThemestring names ending with'Dark', which is brittle; consider basing this on a centralized theme enum/config or explicit mapping so future theme name changes don’t silently break dark detection. - In
_apply_theme_to_html, you always inject a<meta name="color-scheme">tag without checking for an existing one, which can lead to duplicates; consider detecting and updating an existing tag instead of blindly appending.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isDark` getter in `customizer.ts` relies on `uiTheme` string names ending with `'Dark'`, which is brittle; consider basing this on a centralized theme enum/config or explicit mapping so future theme name changes don’t silently break dark detection.
- In `_apply_theme_to_html`, you always inject a `<meta name="color-scheme">` tag without checking for an existing one, which can lead to duplicates; consider detecting and updating an existing tag instead of blindly appending.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="232" />
<code_context>
return default
return locale
+ @staticmethod
+ def _get_request_theme() -> str | None:
+ theme = request.args.get("theme", "").strip()
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the theme handling by refactoring HTML rewriting into clearer single-pass transformations and centralizing theme derivation in a small helper object used across call sites.
You can reduce the theme-related complexity without changing behavior by (1) simplifying the HTML rewriting into fewer, clearer transformations and (2) centralizing theme derivation so it doesn’t get recomputed / threaded in multiple ad‑hoc ways.
### 1. Simplify `_apply_theme_to_html`
You can avoid multiple regex passes and nested branching by transforming the `<html>` and `<head>` tags in one pass each, with local helpers:
```python
@staticmethod
def _apply_theme_to_html(html: str, theme: str) -> str:
# Update <html> tag: strip any existing data-theme and add the new one
def _replace_html_tag(m: re.Match) -> str:
attrs = m.group(1) or ""
# strip existing data-theme from attrs only
attrs = re.sub(
r'\s+data-theme\s*=\s*["\'][^"\']*["\']',
"",
attrs,
flags=re.IGNORECASE,
)
return f"<html{attrs} data-theme=\"{theme}\">"
html = re.sub(
r"<html([^>]*)>",
_replace_html_tag,
html,
count=1,
flags=re.IGNORECASE,
)
# Ensure color-scheme meta in <head>
meta_tag = f'<meta name="color-scheme" content="{theme}">'
def _inject_meta_into_head(m: re.Match) -> str:
return f"{m.group(0)}{meta_tag}"
if re.search(r"<head\b[^>]*>", html, re.IGNORECASE):
html = re.sub(
r"<head\b[^>]*>",
_inject_meta_into_head,
html,
count=1,
flags=re.IGNORECASE,
)
else:
# no <head>: inject a minimal head right after <html ...>
html = re.sub(
r"(<html\b[^>]*>)",
rf"\1<head>{meta_tag}</head>",
html,
count=1,
flags=re.IGNORECASE,
)
return html
```
This keeps all existing semantics but:
- uses one regex for `<html>` with in-place `data-theme` cleanup
- uses one clear path for `<head>` injection instead of multiple separate `search`/`replace` calls
### 2. Centralize theme derivation and reuse it
Right now `_get_request_theme()` is called in multiple places and also reconstructed via `extra_query_params["theme"]`. You can derive the theme once and pass a small object around, reducing scattered logic.
For example, introduce a tiny helper:
```python
@dataclass(frozen=True)
class RequestTheme:
value: str | None
@property
def is_dark(self) -> bool:
return self.value == "dark"
def as_query_param(self) -> dict[str, str]:
return {"theme": self.value} if self.value else {}
```
Then:
```python
@staticmethod
def _get_request_theme() -> RequestTheme:
raw = request.args.get("theme", "").strip()
return RequestTheme(raw if raw in ("dark", "light") else None)
```
Use it in the different call sites instead of re-deriving:
```python
def _get_plugin_page_initial_context(self) -> dict | None:
...
theme = self._get_request_theme()
return {
...
"isDark": theme.is_dark,
}
```
```python
def _prepare_plugin_page_query_params(
self,
plugin_name: str,
page_name: str,
) -> dict[str, str] | None:
params: dict[str, str] = {}
...
if asset_token:
params["asset_token"] = asset_token
theme = self._get_request_theme()
params.update(theme.as_query_param())
return params or None
```
And in `_rewrite_plugin_page_html_assets`, avoid re-reading from `extra_query_params` and use the same theme source:
```python
def _rewrite_plugin_page_html_assets(...):
...
theme = self._get_request_theme()
if theme.value:
rewritten_html = self._apply_theme_to_html(rewritten_html, theme.value)
...
```
This keeps all current behavior (theme query param, `isDark` flag, HTML rewriting) but:
- makes the theme’s lifecycle explicit (derived once per request)
- reduces the number of places that “know” how to interpret `"dark"` / `"light"`
- removes the need to thread raw theme values through multiple helpers implicitly via `extra_query_params`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…en isDark getter, consolidate theme source
- _apply_theme_to_html: strip existing color-scheme meta before injecting to
avoid duplicates; merge data-theme strip+add into single-pass regex callback
- customizer.ts: replace brittle endsWith('Dark') with explicit theme name check
- _rewrite_plugin_page_html: use _get_request_theme() directly instead of
reading theme from extra_query_params
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Overwriting existing
<html data-theme>and<meta name="color-scheme">in_apply_theme_to_html()may unexpectedly override plugin-controlled theming; consider only injecting these when they are absent or making this behavior opt-in for plugins. - The iframe
themequery param is only set on initial load; if you intend the server-side rendered theme to stay in sync after dashboard theme toggles, consider updatingiframeSrc(or reloading the iframe) whenuiThemechanges, not just resending the postMessage context. - The
isDarkgetter incustomizeris hardcoded touiTheme === 'PurpleThemeDark'; if additional dark themes are introduced later this will break theme detection, so it may be safer to derive darkness from a more generic property or an enum of dark themes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Overwriting existing `<html data-theme>` and `<meta name="color-scheme">` in `_apply_theme_to_html()` may unexpectedly override plugin-controlled theming; consider only injecting these when they are absent or making this behavior opt-in for plugins.
- The iframe `theme` query param is only set on initial load; if you intend the server-side rendered theme to stay in sync after dashboard theme toggles, consider updating `iframeSrc` (or reloading the iframe) when `uiTheme` changes, not just resending the postMessage context.
- The `isDark` getter in `customizer` is hardcoded to `uiTheme === 'PurpleThemeDark'`; if additional dark themes are introduced later this will break theme detection, so it may be safer to derive darkness from a more generic property or an enum of dark themes.
## Individual Comments
### Comment 1
<location path="astrbot/dashboard/routes/plugin.py" line_range="232" />
<code_context>
return default
return locale
+ @staticmethod
+ def _get_request_theme() -> str | None:
+ theme = request.args.get("theme", "").strip()
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring theme handling and related helpers to centralize per-request state and make HTML and query param updates more linear and readable.
You can keep all functionality while trimming some complexity in a few focused spots.
### 1. Centralize theme retrieval per request
You call `_get_request_theme()` in three places. Memoizing per request reduces scattered parsing and makes intent clearer:
```python
from flask import g
@staticmethod
def _get_request_theme() -> str | None:
if hasattr(g, "_theme"):
return g._theme # type: ignore[attr-defined]
theme = request.args.get("theme", "").strip()
g._theme = theme if theme in ("dark", "light") else None # type: ignore[attr-defined]
return g._theme # type: ignore[attr-defined]
```
This keeps the API unchanged but avoids re-parsing and helps future readers understand “there is one theme per request”.
### 2. Simplify `isDark` derivation
You can drop the ternary; the semantics are equivalent:
```python
theme = self._get_request_theme()
return {
...
"i18n": plugin_i18n,
"isDark": theme == "dark",
}
```
### 3. Make `_apply_theme_to_html` more linear and localized
You can keep the regex-based approach but make it easier to follow by doing a single pass per concern and avoiding nested callbacks where possible:
```python
@staticmethod
def _apply_theme_to_html(html: str, theme: str) -> str:
# Ensure <html> has data-theme
def _replace_html_tag(m: re.Match) -> str:
tag = m.group(0)
# strip any existing data-theme on the tag only
tag = re.sub(
r'\s+data-theme\s*=\s*["\'][^"\']*["\']',
"",
tag,
flags=re.IGNORECASE,
)
# inject data-theme before closing ">"
return re.sub(r">\s*$", f' data-theme="{theme}">', tag)
html = re.sub(
r"<html\b[^>]*>",
_replace_html_tag,
html,
count=1,
flags=re.IGNORECASE,
)
# Ensure <meta name="color-scheme"> exists with correct content
meta_tag = f'<meta name="color-scheme" content="{theme}">'
# remove existing color-scheme meta tags
html = re.sub(
r'<meta\s[^>]*name\s*=\s*["\']color-scheme["\'][^>]*>',
"",
html,
flags=re.IGNORECASE,
)
head_match = re.search(r"<head\b[^>]*>", html, re.IGNORECASE)
if head_match:
# append into existing <head>
replacement = head_match.group(0) + meta_tag
html = html.replace(head_match.group(0), replacement, 1)
else:
# insert a minimal <head> if none exists
html = re.sub(
r"(<html\b[^>]*>)",
rf"\1<head>{meta_tag}</head>",
html,
count=1,
flags=re.IGNORECASE,
)
return html
```
This keeps your behavior (strip old `data-theme` and `color-scheme` meta, then inject new ones) but avoids manipulating raw attribute slices and makes each step (“fix `<html>`”, “fix `<meta>`”) more self-contained.
### 4. Reduce branching in `_prepare_plugin_page_query_params`
You can keep the `theme` propagation but simplify the function by computing values once and constructing the dict in a single place:
```python
def _prepare_plugin_page_query_params(
self,
plugin_name: str,
page_name: str,
) -> dict[str, str] | None:
asset_token = request.args.get("asset_token", "").strip()
if not asset_token:
asset_token = self._issue_plugin_page_asset_token(plugin_name, page_name) or ""
theme = self._get_request_theme()
if not asset_token and not theme:
return None
params: dict[str, str] = {}
if asset_token:
params["asset_token"] = asset_token
if theme:
params["theme"] = theme
return params
```
This keeps all existing behavior but reads more linearly: compute values → early return if nothing → build dict.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- isDark 推导简化为 theme == "dark"(None == "dark" 为 False,去掉多余三元表达式) - _prepare_plugin_page_query_params 改为线性构建,先计算再构建字典 - isDark getter 改用显式 DARK_THEMES Set,替代硬编码单值比较
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/views/PluginPagePage.vue" line_range="439-441" />
<code_context>
plugin.value = pluginData;
page.value = pageEntry;
- iframeSrc.value = pageEntry.content_path;
+ const contentUrl = new URL(pageEntry.content_path, window.location.origin);
+ contentUrl.searchParams.set('theme', themeParam.value);
+ iframeSrc.value = contentUrl.pathname + contentUrl.search + contentUrl.hash;
} catch (error) {
errorMessage.value =
</code_context>
<issue_to_address>
**issue:** Preserving only pathname/search/hash changes behavior for absolute URLs and may break cross-origin plugin pages.
Previously `iframeSrc` used `pageEntry.content_path` directly, so absolute URLs (including cross-origin) worked as-is. Now, building a `URL` and then assigning only `pathname + search + hash` forces everything to the current origin.
If plugin pages can be hosted on other origins or subpaths where the origin matters, this will rewrite and break those URLs. To avoid that while still injecting the `theme` param for same-origin/relative paths, consider:
- Only rewriting via `new URL` when `pageEntry.content_path` is relative, or
- For absolute URLs, preserving `contentUrl.href` (or at least the original origin) instead of truncating to `pathname + search + hash`.
</issue_to_address>
### Comment 2
<location path="astrbot/dashboard/routes/plugin.py" line_range="238" />
<code_context>
+ return theme if theme in ("dark", "light") else None
+
+ @staticmethod
+ def _apply_theme_to_html(html: str, theme: str) -> str:
+ def _replace_html_tag(m: re.Match) -> str:
+ attrs = m.group(1) or ""
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new theme-handling logic into smaller helpers and a shared theme context function to simplify control flow and make behavior easier to follow and test.
You can reduce the new theme-related complexity with two small refactors without changing behavior:
1. **Split `_apply_theme_to_html` into focused helpers**
Right now it does HTML `data-theme` and `<meta name="color-scheme">` handling in one method with nested regex. You can keep the string/regex approach but separate responsibilities so each piece is easier to reason about and test.
Example:
```python
@staticmethod
def _set_html_data_theme(html: str, theme: str) -> str:
def _replace_html_tag(m: re.Match) -> str:
attrs = m.group(1) or ""
attrs = re.sub(
r'\s+data-theme\s*=\s*["\'][^"\']*["\']',
"",
attrs,
flags=re.IGNORECASE,
)
return f'<html{attrs} data-theme="{theme}">'
return re.sub(
r"<html(\b[^>]*)>",
_replace_html_tag,
html,
count=1,
flags=re.IGNORECASE,
)
@staticmethod
def _set_head_color_scheme_meta(html: str, theme: str) -> str:
meta_tag = f'<meta name="color-scheme" content="{theme}">'
html = re.sub(
r'<meta\s[^>]*name\s*=\s*["\']color-scheme["\'][^>]*>',
"",
html,
flags=re.IGNORECASE,
)
head_match = re.search(r"<head\b[^>]*>", html, re.IGNORECASE)
if head_match:
return html.replace(head_match.group(0), f"{head_match.group(0)}{meta_tag}", 1)
return re.sub(
r"(<html\b[^>]*>)",
rf"\1<head>{meta_tag}</head>",
html,
count=1,
flags=re.IGNORECASE,
)
@staticmethod
def _apply_theme_to_html(html: str, theme: str) -> str:
html = self._set_html_data_theme(html, theme)
html = self._set_head_color_scheme_meta(html, theme)
return html
```
That keeps all current behavior but makes each step linear and self-describing.
2. **Centralize theme extraction to avoid scattering**
`_get_request_theme` is called in multiple places (`_get_plugin_page_initial_context`, `_prepare_plugin_page_query_params`, `render_plugin_page_html`). You can compute it once per request/path and reuse it, which also makes the propagation path clearer.
For example, introduce a small helper that returns both `theme` and `is_dark` and reuse it:
```python
@staticmethod
def _get_theme_context() -> dict[str, Any]:
theme = request.args.get("theme", "").strip()
theme = theme if theme in ("dark", "light") else None
return {
"theme": theme,
"is_dark": theme == "dark",
}
```
Then:
```python
def _get_plugin_page_initial_context(self) -> dict | None:
...
theme_ctx = self._get_theme_context()
return {
"pluginName": plugin.name,
...
"i18n": plugin_i18n,
"isDark": theme_ctx["is_dark"],
}
```
```python
def _prepare_plugin_page_query_params(...):
...
theme_ctx = self._get_theme_context()
theme = theme_ctx["theme"]
if not asset_token and not theme:
return None
params: dict[str, str] = {}
if asset_token:
params["asset_token"] = asset_token
if theme:
params["theme"] = theme
return params
```
```python
rewritten_html = _HTML_ASSET_ATTR_RE.sub(replace_attr, html_text)
theme_ctx = self._get_theme_context()
if theme_ctx["theme"]:
rewritten_html = self._apply_theme_to_html(rewritten_html, theme_ctx["theme"])
```
This keeps functionality intact but removes repeated `theme` parsing and makes the theme flow (request → context/query params → HTML) easier to track.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a4c4a7d to
9bd38ca
Compare
Plugin pages embedded via iframe had no awareness of the dashboard's light/dark theme state, causing two problems:
This PR adds an end-to-end theme sync mechanism so plugin pages automatically match the dashboard theme on load and react to live theme switching, with the initial theme applied server-side to eliminate flash.
Modifications / 改动点
Backend (plugin.py): Added theme query parameter propagation through the plugin page asset pipeline. The _get_plugin_page_initial_context now includes isDark. The HTML rewriter injects data-theme on and a tag server-side to prevent initial flash.
Bridge SDK (plugin_page_bridge.js): applyContext() now sets data-theme on document.documentElement when context includes isDark, enabling live theme switching.
Frontend (PluginPagePage.vue): Appends &theme=dark/light to iframe URL and includes isDark in postMessage context. Watches uiTheme to re-send context on theme toggle.
Store (customizer.ts): Added isDark getter.
Tests: Added tests for isDark in bridge SDK initial context (dark/light/absent/invalid params) and theme propagation in rewritten HTML (data-theme, color-scheme meta).
Docs: Added light/dark theme adaptation section to plugin-pages guide (zh + en).
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Testing on the
astrbot_plugin_livingmemoryplugin showed that switching between light and dark themes using AstrBot's built-in theme switching function resulted in the Livingmemory WebUI switching themes accordingly. Furthermore, the WebUI could be switched back to the opposite theme without affecting the display of the AstrBot WebUI.The current default behavior is to maintain the same light/dark theme as the AstrBot WebUI when entering the plugin's WebUI.
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Synchronize the dashboard’s light/dark theme with plugin pages and eliminate initial theme flash for iframe-embedded plugin UIs.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by Sourcery
Synchronize the dashboard’s light/dark theme with iframe-embedded plugin pages so they load with the correct theme and stay in sync with live theme changes.
New Features:
Enhancements:
Documentation:
Tests: