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.

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

refactor(theme-generator): adapt for mobile and mini-program #602

Merged
merged 46 commits into from
Apr 2, 2025
Merged

Conversation

RylanBot
Copy link
Collaborator

@RylanBot RylanBot commented Mar 14, 2025

🤔 这个 PR 的性质是?

  • 日常 bug 修复
  • 新特性提交
  • 文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • CI/CD 改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他

🔗 相关 Issue

💡 需求背景和解决方案

📝 更新日志

  • refactor(theme-generator): adapt for mobile and mini-program

  • 本条 PR 不需要纳入 Changelog

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

@tencent-adm
Copy link
Member

tencent-adm commented Mar 14, 2025

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ uyarn
❌ RylanBot
You have signed the CLA already but the status is still pending? Let us recheck it.

@liweijie0812 liweijie0812 requested a review from Copilot March 14, 2025 13:23
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the theme generation logic by splitting the light and dark CSS into separate parts. The key changes include:

  • Introducing the extractThemeString function to separate dark and light CSS from the combined stylesheet.
  • Updating generateNewTheme to use textContent instead of innerText and applying the new extraction logic for built-in themes.
  • Modifying Generator.vue to generate the theme using a dynamic default value instead of a hardcoded hex color.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/theme-generator/src/common/utils/index.js Added extractThemeString function for splitting CSS and updated generateNewTheme to use textContent.
packages/theme-generator/src/Generator.vue Replaced the hardcoded theme color with defaultTheme.value in the mounted hook.
Comments suppressed due to low confidence (1)

packages/theme-generator/src/common/utils/index.js:175

  • Since the theme updates now use textContent, please consider using textContent consistently in the download function to avoid potential discrepancies between innerText and textContent.
let cssVariablesString = styleSheet?.innerText?.replaceAll(`[theme-color="${customTheme}"]`, '');

@RylanBot RylanBot removed the 🏃🏻 in progress someone is handling label Mar 19, 2025
@RylanBot RylanBot changed the title chore(theme-generator): split light and dark CSS files into two chore(theme-generator): split theme variables into files by device Mar 19, 2025
@RylanBot RylanBot added the 🏃🏻 in progress someone is handling label Mar 19, 2025
@RylanBot RylanBot removed the 🏃🏻 in progress someone is handling label Mar 20, 2025
@RylanBot RylanBot changed the title chore(theme-generator): split theme variables into files by device refactor(theme-generator): split theme variables into files by device Mar 25, 2025
@RylanBot RylanBot changed the title refactor(theme-generator): split theme variables into files by device refactor(theme-generator): adapt for mobile and mini-program Mar 31, 2025
@liweijie0812 liweijie0812 requested a review from Copilot April 1, 2025 04:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the theme-generator to adapt it for mobile and mini-program platforms while standardizing code style and function naming. Key changes include filtering out the size configuration tab on mobile devices, unifying theme generation function calls with device support, and reformatting code to use consistent styling and quoting.

Reviewed Changes

Copilot reviewed 51 out of 62 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/theme-generator/src/common/SwitchTabs/index.vue Updates the tab loop to use a filtered list of tabs on mobile devices.
packages/theme-generator/src/common/ColorPicker/index.vue Simplifies component structure and adjusts prop passing and quoting style.
packages/theme-generator/src/color-panel/utils/const.js Refactors import quoting and order while preserving constant values.
packages/theme-generator/src/color-panel/components/ColorContent/index.vue Updates theme color comparisons to use case-insensitive matching and standardizes theme generation.
packages/theme-generator/src/color-panel/components/ColorCollapse.vue Adjusts markup and formatting for clarity.
packages/theme-generator/src/color-panel/components/ColorColumn/index.vue Refines inline style calculations and improves consistency.
packages/theme-generator/src/Generator.vue Passes the device prop to theme generation functions and updates import paths and styles.
packages/theme-generator/README.md & README-zh_CN.md Documents the new device parameter usage for mobile and mini-program support.
Files not reviewed (11)
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/TDesign/dark.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/TDesign/light.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/common/font.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/common/radius.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/common/shadow.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/mobile/common/spacer.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/web/TCloud/dark.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/web/TCloud/light.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/web/TDesign/dark.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/web/TDesign/light.css: Language not supported
  • packages/theme-generator/src/common/Themes/built-in/css/web/common/font.css: Language not supported
Comments suppressed due to low confidence (3)

packages/theme-generator/src/color-panel/components/ColorContent/index.vue:658

  • [nitpick] The function 'handleNewColorGeneration' replaces the previous 'generateNewTheme' call. Consider standardizing the naming convention for theme generation functions across the codebase to improve clarity.
handleNewColorGeneration(hex) {

packages/theme-generator/src/Generator.vue:66

  • [nitpick] Ensure that the 'device' prop is validated and defaults to an accepted value to prevent potential issues when generating the theme on different platforms.
generateNewTheme(DEFAULT_THEME.value, undefined, this.device);

packages/theme-generator/src/color-panel/components/ColorColumn/index.vue:30

  • [nitpick] Consider moving the inline style calculation for positioning the active tab into a computed property to enhance template readability and maintainability.
top: `${flattenPalette.filter((v) => !!v.name).findIndex((v) => v.idx === activeIdx) * 44 + 4}px`,

@uyarn uyarn merged commit 37eb9f5 into main Apr 2, 2025
3 of 4 checks passed
@uyarn uyarn deleted the chore/theme branch April 2, 2025 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants