fix(site): remove switch theme button #2995
Conversation
WalkthroughThe pull request removes the dark mode toggle functionality from the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
examples/sites/public/static/js/design-common.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough此PR移除了官网页面上的切换主题按钮。开发者需要在开发时手动在HTML标签上添加 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -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 | |||
There was a problem hiding this comment.
移除了切换主题按钮的代码，这意味着用户将无法通过UI切换主题。确保这是预期的行为，并且在文档中有说明如何手动切换主题。
WalkthroughThis PR removes the Toggle Theme button on the official website page. Developers need to manually add `class="dark" on HTML tags during development to switch to dark themes. Changes
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/sites/src/assets/custom-markdown.css (1)
110-120: Consider using CSS variables for consistent color themingWhile you're using
var(--tv-color-text)for the code text color, the nested selectors use hardcoded color values. Consider using CSS variables for all colors to maintain consistency and make future theme adjustments easier..dark.dark .markdown-body code { color: var(--tv-color-text); .token.operator { background-color: transparent; } .hljs-string { - color: #6f42c1; + color: var(--color-prettylights-syntax-entity, #6f42c1); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/sites/src/assets/images/leftMenu/directives-custom-instruction.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
examples/sites/src/assets/custom-markdown.css(1 hunks)examples/sites/src/assets/markdown.less(0 hunks)examples/sites/src/main.js(1 hunks)examples/sites/src/views/components-doc/components/demo.vue(1 hunks)examples/sites/src/views/docs/docs.vue(0 hunks)
💤 Files with no reviewable changes (2)
- examples/sites/src/assets/markdown.less
- examples/sites/src/views/docs/docs.vue
✅ Files skipped from review due to trivial changes (1)
- examples/sites/src/views/components-doc/components/demo.vue
🔇 Additional comments (4)
examples/sites/src/main.js (1)
10-10:✅ Verification successful
Removing default highlight.js styling might affect code highlighting appearance
This change comments out the import for highlight.js default styles, which aligns with the PR objective of removing the theme switch button. However, it's important to verify that code highlighting still appears correctly in light mode without these default styles.
🏁 Script executed:
#!/bin/bash # Check if highlight.js styling is handled elsewhere in the codebase echo "Checking for other highlight.js style imports:" rg -i "highlight.js.*style" --type css --type less rg -i "highlight.js.*css" --type js --type vue echo -e "\nChecking for custom highlighting styles that might replace the default:" rg -i "\.hljs" --type css --type lessLength of output: 4335
Verified: Custom Highlighting Styles in Place
- The removal of the default
highlight.jsstyle import (i.e., commenting outimport 'highlight.js/styles/default.css') aligns with the PR’s objective and is acceptable.- Verification shows that custom styling for code highlighting (using
.hljsselectors) is defined in the codebase (e.g., inexamples/sites/src/assets/custom-markdown.cssand within the Less files underpackages/theme/src/rich-text-editorandpackages/theme-saas/src/rich-text-editor).- These custom styles should ensure that code highlighting remains consistent in light mode despite the removal of the default styles.
examples/sites/src/assets/custom-markdown.css (3)
98-100: Removing text shadows from code elements improves readabilityThis change ensures code elements in the markdown body don't have text shadows, which improves readability, especially in dark mode. The use of
!importantensures this style takes precedence over any other conflicting styles.
106-108: Dark mode pre element styling enhances code block readabilitySetting the background color of
preelements to a dark gray (#1a1a1a) in dark mode provides good contrast for code blocks. This improves readability for code samples when in dark mode.
102-104:❓ Verification inconclusive
Verify the double class selector
.dark.darkis intentionalThe double class selector
.dark.darkis unusual and might be a typo. If intentional, it suggests a need for higher specificity to override other styles. Consider using a single.darkclass with higher specificity selectors if needed.
🏁 Script executed:
#!/bin/bash # Check for other occurrences of .dark.dark in the codebase echo "Checking for double .dark.dark class usage:" rg "\.dark\.dark" --type css --type less --type vue echo -e "\nChecking for relevant dark theme class usage in Vue components:" rg "class=\".*dark.*\"" --type vueLength of output: 384
Action: Verify the usage of the double class selector
.dark.darkThe initial script encountered issues searching Vue files (the
--type vueflag wasn’t recognized). This makes it unclear whether.dark.darkis used elsewhere or if this occurrence inexamples/sites/src/assets/custom-markdown.cssis accidental. To clarify this, please run the following updated script that uses glob patterns instead:#!/bin/bash echo "Checking for .dark.dark in CSS, LESS, and Vue files using updated glob patterns:" rg "\.dark\.dark" --glob "*.{css,less,vue}" echo -e "\nChecking for relevant dark theme class usage in Vue components using updated glob patterns:" rg "class=\".*dark.*\"" --glob "*.vue"Once you’ve confirmed whether the double class selector appears intentionally elsewhere or is unique to this file, please update the CSS accordingly—either by documenting the intentional higher specificity or by refactoring it if it was a typo.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
examples/sites/src/App.vue(1 hunks)examples/sites/src/views/components-doc/common.vue(1 hunks)examples/sites/src/views/docs/docs.vue(1 hunks)examples/sites/src/views/overview.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/sites/src/views/docs/docs.vue
🔇 Additional comments (3)
examples/sites/src/App.vue (1)
34-34: Support for dark theme addedThe
allowDarkTheme: trueparameter is added to the TDCommon configuration to support the dark mode styling. This aligns with the PR objective of removing the theme switch button and managing themes through manually addingclass="dark"to HTML tags during development.examples/sites/src/views/overview.vue (1)
162-162: Dark theme support added to footerThe
allowDarkTheme: trueparameter is added to the TDCommon configuration for the footer component, which aligns with the PR objective of supporting dark theme while removing the toggle functionality.examples/sites/src/views/components-doc/common.vue (1)
165-165: Consistent dark theme implementationThe addition of
allowDarkTheme: trueparameter is consistent with changes in other files and supports the PR's goal of implementing dark theme styling while removing the theme toggle button.
| @@ -162,7 +162,7 @@ watch( | |||
| onMounted(() => { | |||
| loadPage() | |||
| // 加载公共尾部 | |||
| const common = new window.TDCommon(['#footer'], {}) | |||
| const common = new window.TDCmmon(['#footer'], { allowDarkTheme: true }) | |||
There was a problem hiding this comment.
Fix typo in constructor name
There's a typo in the constructor name - TDCmmon should be TDCommon. This will cause the footer to not render correctly.
Apply this fix:
- const common = new window.TDCmmon(['#footer'], { allowDarkTheme: true })
+ const common = new window.TDCommon(['#footer'], { allowDarkTheme: true })📝 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.
| const common = new window.TDCmmon(['#footer'], { allowDarkTheme: true }) | |
| const common = new window.TDCommon(['#footer'], { allowDarkTheme: true }) |
| @@ -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> | |||
There was a problem hiding this comment.
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:
- It uses hardcoded inline styling (
margin-left:150px) which is generally not recommended for maintainability - The button text is in Chinese ("切换主题") without internationalization support
- 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.
| <button id="switchTheme" style="margin-left:150px">切换主题</button> |
|
|
||
| document.getElementById('switchTheme').addEventListener('click', () => { | ||
| document.querySelector('html').classList.toggle('dark') | ||
| }) |
There was a problem hiding this comment.
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.
| document.getElementById('switchTheme').addEventListener('click', () => { | |
| document.querySelector('html').classList.toggle('dark') | |
| }) | |
| // Theme toggling functionality removed per PR objective. |
| 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'); | |
| } |
PR
移除官网页面上的，切换主题的按钮。
开发时需要切暗色主题，手动在 html 标签上添加 class="dark"
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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