-
Notifications
You must be signed in to change notification settings - Fork 338
fix(mobile): add mobile dark theme switching #4033
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
base: dev
Are you sure you want to change the base?
Conversation
WalkthroughAdds a dark theme import and runtime logic that observes host document class changes to inject or remove a dark stylesheet and the Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Host Document
participant Observer as MutationObserver
participant Iframe as Preview Iframe
participant IframeDoc as Iframe Document
Host->>Observer: start observing class changes
Observer-->>Host: detects class add/remove
Host->>Iframe: call onIframeLoad or removal logic
Iframe->>IframeDoc: attempt access to documentElement
alt access allowed
IframeDoc->>IframeDoc: add/remove 'dark' class
IframeDoc->>IframeDoc: create/remove <link id="theme-style-link"> for mobile-dark-theme.css
else access denied
IframeDoc-->>Host: access error (caught & handled)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/sites/src/views/components-doc/mobile.vue (1)
23-24: Move MutationObserver setup into lifecycle hooks and disconnect on unmount.The observer is created and started at module initialization (lines 81-84) but never disconnected. After the component unmounts during navigation, the observer continues running and its callback attempts to access
iframeRef.value(lines 67, 70), which is now invalid. This causes memory leaks and runtime errors.Wrap
observer.observe()inonMounted()and callobserver.disconnect()inonBeforeUnmount(). Update the import at line 23 to includeonMountedandonBeforeUnmountfrom Vue.
🤖 Fix all issues with AI agents
In `@examples/sites/src/views/components-doc/mobile.vue`:
- Around line 64-100: The onIframeLoad/observer logic is adding duplicate theme
links and not fully reverting the iframe to light mode; update onIframeLoad (and
the MutationObserver callback) to: check iframeRef and resolve iframeDocument
safely, before appending look for an existing element with id 'theme-style-link'
and replace or skip creating a new one, and when handling the else (light mode)
branch remove any existing 'theme-style-link' elements (not just one) and also
remove the 'dark' class from iframeDocument.documentElement so the iframe truly
switches back to light; use the unique symbols iframeRef, onIframeLoad, and the
id 'theme-style-link' to locate and change the code.
- Around line 94-97: The href for the created link element is hardcoded to
'/public/...', which breaks when BASE_URL is non-root; update the code that sets
link.href (the link created via iframeDocument.createElement in this block) to
use the existing baseUrl variable and point to
'static/css/mobile-dark-theme.css' (no leading '/public/') so the asset resolves
under the configured base (e.g., link.href = baseUrl +
'static/css/mobile-dark-theme.css').
- Line 14: The iframe element (id "iframeDom", ref "iframeRef", :src
"iframeUrl") needs an `@load` handler so the initial theme is applied when the
iframe loads; add `@load`="onIframeLoad" (or similar) and implement onIframeLoad
to invoke the same theming routine used by the MutationObserver callback (the
function that injects the dark CSS into the iframe) by checking
document.documentElement.classList.contains('dark') and applying/removing the
dark styles to iframeRef.value.contentDocument accordingly so pages that start
in dark mode are themed immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@examples/sites/src/views/components-doc/mobile.vue`:
- Around line 64-85: The MutationObserver created as observer (which references
iframeRef and onIframeLoad) is set up at module level and never torn down; move
the observer creation and observer.observe(...) into the component lifecycle by
creating it inside onMounted and store it in a local variable (e.g., observer),
and add an onBeforeUnmount hook that calls observer.disconnect() to stop
observation and release references to iframeRef/onIframeLoad; ensure the same
logic for toggling iframe theme remains inside the mounted callback so the
observer callback can access iframeRef and onIframeLoad.
| const observer = new MutationObserver(() => { | ||
| const isDarkMode = document.documentElement.classList.contains('dark') | ||
| if (isDarkMode) { | ||
| onIframeLoad() | ||
| } else { | ||
| try { | ||
| const iframeDocument = iframeRef.value.contentDocument || iframeRef.value.contentWindow.document | ||
| iframeDocument.documentElement.classList.remove('dark') | ||
| const linkElement = iframeDocument.getElementById('theme-style-link') | ||
| if (linkElement) { | ||
| linkElement.remove() | ||
| } | ||
| } catch (error) { | ||
| console.error('无法访问 iframe:', error) | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| observer.observe(document.documentElement, { | ||
| attributes: true, | ||
| attributeFilter: ['class'] | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd mobile.vueRepository: opentiny/tiny-vue
Length of output: 111
🏁 Script executed:
cat -n examples/sites/src/views/components-doc/mobile.vue | head -100Repository: opentiny/tiny-vue
Length of output: 3676
🌐 Web query:
Vue 3 script setup MutationObserver cleanup onMounted onBeforeUnmount
💡 Result:
In Vue 3 <script setup>, create the MutationObserver after the DOM exists (typically in onMounted), and disconnect it when the component is destroyed (via onBeforeUnmount / onUnmounted). Vue lifecycle hooks must be registered synchronously during setup / <script setup>. [1][2]
<script setup lang="ts">
import { ref, onMounted, onBeforeUnmount } from 'vue'
const el = ref<HTMLElement | null>(null)
let observer: MutationObserver | null = null
onMounted(() => {
if (!el.value) return
observer = new MutationObserver((mutations) => {
// handle mutations
})
observer.observe(el.value, {
childList: true,
subtree: true,
attributes: true,
})
})
onBeforeUnmount(() => {
observer?.disconnect()
observer = null
})
</script>
<template>
<div ref="el"></div>
</template>Alternative: use a composable that automatically disconnects on disposal/unmount (e.g., via onScopeDispose). [3]
Sources: [1] Vue Lifecycle Hooks Guide, [2] Vue Composition API Lifecycle Hooks, [3] Vuetify useMutationObserver cleanup notes.
Disconnect the MutationObserver on unmount to avoid memory leaks.
The observer is created at module level and never disconnected, so it persists after component destruction and retains stale references. Move setup into onMounted and disconnect in onBeforeUnmount.
🔧 Suggested fix
-import { ref } from 'vue'
+import { ref, onMounted, onBeforeUnmount } from 'vue'
@@
-const observer = new MutationObserver(() => {
- const isDarkMode = document.documentElement.classList.contains('dark')
- if (isDarkMode) {
- onIframeLoad()
- } else {
- try {
- const iframeDocument = iframeRef.value.contentDocument || iframeRef.value.contentWindow.document
- iframeDocument.documentElement.classList.remove('dark')
- const linkElement = iframeDocument.getElementById('theme-style-link')
- if (linkElement) {
- linkElement.remove()
- }
- } catch (error) {
- console.error('无法访问 iframe:', error)
- }
- }
-})
-
-observer.observe(document.documentElement, {
- attributes: true,
- attributeFilter: ['class']
-})
+let observer
+onMounted(() => {
+ observer = new MutationObserver(() => {
+ const isDarkMode = document.documentElement.classList.contains('dark')
+ if (isDarkMode) {
+ onIframeLoad()
+ } else {
+ try {
+ const iframeDocument = iframeRef.value.contentDocument || iframeRef.value.contentWindow.document
+ iframeDocument.documentElement.classList.remove('dark')
+ const linkElement = iframeDocument.getElementById('theme-style-link')
+ if (linkElement) {
+ linkElement.remove()
+ }
+ } catch (error) {
+ console.error('无法访问 iframe:', error)
+ }
+ }
+ })
+
+ observer.observe(document.documentElement, {
+ attributes: true,
+ attributeFilter: ['class']
+ })
+})
+
+onBeforeUnmount(() => {
+ observer?.disconnect()
+})🤖 Prompt for AI Agents
In `@examples/sites/src/views/components-doc/mobile.vue` around lines 64 - 85, The
MutationObserver created as observer (which references iframeRef and
onIframeLoad) is set up at module level and never torn down; move the observer
creation and observer.observe(...) into the component lifecycle by creating it
inside onMounted and store it in a local variable (e.g., observer), and add an
onBeforeUnmount hook that calls observer.disconnect() to stop observation and
release references to iframeRef/onIframeLoad; ensure the same logic for toggling
iframe theme remains inside the mounted callback so the observer callback can
access iframeRef and onIframeLoad.
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
在vue-docs的mobile移动端添加iframe暗色主题切换
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.