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.

Sign up for GitHub

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

fix(site): refactor the implementation mode of switch dark theme #2995

Merged
merged 5 commits into from
Mar 4, 2025
+45 −74
Merged

fix(site): refactor the implementation mode of switch dark theme #2995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0fe2122
fix(site): remove switch theme button
shenjunjian Mar 3, 2025
bf6472a
fix(site): fix some wrong colors
shenjunjian Mar 3, 2025
1799218
fix(site): add { allowDarkTheme:true } to TDCommon
shenjunjian Mar 3, 2025
3677496
fix(site): add mock button for switch theme
shenjunjian Mar 3, 2025
af9383c
fix(site): fix reviews
shenjunjian Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions examples/sites/public/static/js/design-common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const headerHtml = `
<span class="header-detail-name">TinyVue</span>
<div class="nav-menus nav-menus-left"></div>
<button id="switchTheme" style="margin-left:150px">切换主题</button>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

This button addition contradicts the PR objective to remove the theme switch.

According to the PR objective, the goal is to remove the theme switch button from the official website, but this code is adding a new one instead. Additionally, I see the following issues with this button:

  1. It uses hardcoded inline styling (margin-left:150px) which is generally not recommended for maintainability
  2. The button text is in Chinese ("切换主题") without internationalization support
  3. The button lacks accessibility attributes (like aria-label)

If the intent is to remove theme switching as stated in the PR objectives, consider removing this button:

-      <button id="switchTheme" style="margin-left:150px">切换主题</button>

If theme switching should be kept but implemented differently, consider proper styling and accessibility:

-      <button id="switchTheme" style="margin-left:150px">切换主题</button>
+      <button id="switchTheme" class="theme-toggle-button" aria-label="Toggle dark/light theme">切换主题</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<button id="switchTheme" style="margin-left:150px">切换主题</button>

</div>
</div>
</div>`
Expand Down Expand Up @@ -147,6 +148,10 @@ class DesignCommon {
link.href = '/static/css/design-common.css'
link.rel = 'stylesheet'
document.head.append(link)

document.getElementById('switchTheme').addEventListener('click', () => {
document.querySelector('html').classList.toggle('dark')
})
Comment on lines +151 to +154
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Theme toggle logic contradicts PR objective and lacks persistence.

The PR objective states that the theme switch functionality should be removed, requiring developers to "manually add class='dark' to HTML tags during development." However, this event listener is adding automated theme toggling that affects all users.

Additionally, this implementation doesn't persist the user's theme preference across page reloads, which leads to a poor user experience if this is intended as a user-facing feature.

If the intent is to remove theme switching as stated in the PR objectives, this event listener should be removed:

-    document.getElementById('switchTheme').addEventListener('click', () => {
-      document.querySelector('html').classList.toggle('dark')
-    })

If theme switching should be kept, consider adding persistence:

     document.getElementById('switchTheme').addEventListener('click', () => {
-      document.querySelector('html').classList.toggle('dark')
+      const html = document.querySelector('html');
+      const isDark = html.classList.toggle('dark');
+      localStorage.setItem('theme-preference', isDark ? 'dark' : 'light');
     })
+    
+    // Apply saved theme preference on load
+    const savedTheme = localStorage.getItem('theme-preference');
+    if (savedTheme === 'dark') {
+      document.querySelector('html').classList.add('dark');
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
document.getElementById('switchTheme').addEventListener('click', () => {
document.querySelector('html').classList.toggle('dark')
})
// Theme toggling functionality removed per PR objective.
Suggested change
document.getElementById('switchTheme').addEventListener('click', () => {
document.querySelector('html').classList.toggle('dark')
})
document.getElementById('switchTheme').addEventListener('click', () => {
const html = document.querySelector('html');
const isDark = html.classList.toggle('dark');
localStorage.setItem('theme-preference', isDark ? 'dark' : 'light');
});
// Apply saved theme preference on load
const savedTheme = localStorage.getItem('theme-preference');
if (savedTheme === 'dark') {
document.querySelector('html').classList.add('dark');
}

}
renderFooter() {
document.getElementById('footer').innerHTML = footerHtml
Expand Down
1 change: 1 addition & 0 deletions examples/sites/src/App.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default defineComponent({
onMounted(() => {
// 加载header
const common = new window.TDCommon(['#header'], {
allowDarkTheme: true,
searchConfig: {
show: true
},
Expand Down
23 changes: 23 additions & 0 deletions examples/sites/src/assets/custom-markdown.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,26 @@ body .markdown-body {
--color-attention-subtle: #fff8c5;
--color-danger-fg: #cf222e;
}
.markdown-body code {
text-shadow: none !important;
}

.dark.dark .markdown-body {
background-color: #000;
}

.dark.dark .markdown-body pre {
background-color: #1a1a1a;
}

.dark.dark .markdown-body code {
color: var(--tv-color-text);

.token.operator {
background-color: transparent;
}

.hljs-string {
color: #6f42c1;
}
}
16 changes: 2 additions & 14 deletions examples/sites/src/assets/images/leftMenu/directives-custom-instruction.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 0 additions & 24 deletions examples/sites/src/assets/markdown.less
View file
Open in desktop

This file was deleted.

4 changes: 0 additions & 4 deletions examples/sites/src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ import '@unocss/reset/eric-meyer.css'
import 'prismjs/themes/prism.css'
import 'uno.css'

// highlight默认样式
import 'highlight.js/styles/default.css'

// 只使用markdown的亮色主题
import 'github-markdown-css/github-markdown.css'
import './assets/index.less'
Expand All @@ -17,7 +14,6 @@ import './style.css'
// 覆盖默认的github markdown样式
import './assets/custom-markdown.css'
import './assets/custom-block.less'
import './assets/markdown.less'
import './assets/md-preview.less'

import { i18n } from './i18n/index'
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/src/views/components-doc/common.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ watch(
onMounted(() => {
loadPage()
// 加载公共尾部
const common = new window.TDCommon(['#footer'], {})
const common = new window.TDCommon(['#footer'], { allowDarkTheme: true })
common.renderFooter()
setScrollListener()
})
Expand Down
15 changes: 11 additions & 4 deletions examples/sites/src/views/components-doc/components/demo.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,6 @@ onBeforeUnmount(() => {
}
}
:global(.dark .pc-demo-container.pc-demo-container) {
background-color: #1a1a1a;
border: none;
}
.pc-demo-container {
display: flex;
flex-direction: column;
Expand Down Expand Up @@ -400,3 +396,14 @@ onBeforeUnmount(() => {
padding: 20px 5px;
}
</style>

<style lang="less">
.dark .pc-demo-container.pc-demo-container {
background-color: #1a1a1a;
border: none;
}
.dark .demo-content.demo-content .demo-desc {
color: #b3b3b3;
}
</style>
19 changes: 1 addition & 18 deletions examples/sites/src/views/docs/docs.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ watch([() => router.currentRoute.value, () => router.currentRoute.value.params.d

onMounted(() => {
loadPage()
const common = new window.TDCommon(['#footer'], {})
const common = new window.TDCommon(['#footer'], { allowDarkTheme: true })
common.renderFooter()
})
</script>
Expand All @@ -67,23 +67,6 @@ onMounted(() => {
}
}

.dark .docs-container .markdown-body {
background-color: #000;
}

.dark .docs-container .markdown-body pre {
background-color: #1a1a1a;
}

.dark .docs-container .markdown-body code {
text-shadow: none;
color: var(--tv-color-text);

.token.operator {
background-color: transparent;
}
}

.docs-page-anchor {
.tiny-anchor__affix {
overflow-y: auto;
Expand Down
8 changes: 0 additions & 8 deletions examples/sites/src/views/layout/layout.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
<div class="main-layout ti-hp100 ti-f-c ti-f-box-stretch">
<div class="content-layout ti-fi-1" :has-sider="!isFrame">
<div id="layoutSider" class="layout-sider" :class="{ 'saas-border': templateModeState.isSaas }" v-if="!isFrame">
<div style="padding: 10px 0; text-align: center">
<tiny-button :reset-time="0" type="info" @click="toggleDark()">{{ isDark ? 'dark' : 'light' }}</tiny-button>
</div>
<tiny-tree-menu
Copy link

Choose a reason for hiding this comment

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

移除了切换主题按钮的代码，这意味着用户将无法通过UI切换主题。确保这是预期的行为，并且在文档中有说明如何手动切换主题。

ref="treeMenuRef"
class="main-menu"
Expand Down Expand Up @@ -68,7 +65,6 @@ import { getWord, i18nByKey, appData, appFn, useApiMode, useTemplateMode } from
import useTheme from '@/tools/useTheme'
import FloatSettings from '@/views/components-doc/components/float-settings.vue'
import VersionTip from '@/views/components-doc/components/version-tip.vue'
import { useDark, useToggle } from '@vueuse/core'
export default defineComponent({
name: 'LayoutVue',
Expand Down Expand Up @@ -107,8 +103,6 @@ export default defineComponent({
const getTo = (route, key) => `${import.meta.env.VITE_CONTEXT}${allPath}${lang}/${theme}/${route}${key}`
const isThemeSaas = import.meta.env.VITE_TINY_THEME === 'saas'
const isDark = useDark()
const toggleDark = useToggle(isDark)
const changeLanguage = () => {
appFn.toggleLang()
Expand Down Expand Up @@ -197,8 +191,6 @@ export default defineComponent({
getWord,
i18nByKey,
isThemeSaas,
isDark,
toggleDark,
isShowFilter
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/sites/src/views/overview.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export default defineComponent({
}
onMounted(() => {
const common = new window.TDCommon(['#footer'], {})
const common = new window.TDCommon(['#footer'], { allowDarkTheme: true })
common.renderFooter()
})
return { ...toRefs(state), ...fn, TinyInput, noDataSvg, searchSvg, isZhCn, getWord, i18nByKey, pubUrl }
Expand Down
Loading