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: add global scss #2798

Merged
merged 3 commits into from
Nov 28, 2024
Merged

Conversation

oasis-cloud
Copy link
Collaborator

@oasis-cloud oasis-cloud commented Nov 27, 2024

增加全局样式表,用于组件的通用样式设定。
主要包含:
主题声明
字体

Summary by CodeRabbit

  • 新特性

    • 引入了新的字体样式表和定制滚动条样式。
    • 增强了.demo类的样式,支持多种修饰符,提升了视觉效果。
    • 新增了对黑暗主题和右到左(RTL)布局的支持。
  • 样式更新

    • 改进了应用程序中各个元素的样式,包括导航栏和卡片组件。
    • 定义了新的背景和布局样式,提升了整体用户体验。

Copy link

coderabbitai bot commented Nov 27, 2024

Walkthrough

本次更改涉及多个样式文件的更新,主要包括在 app.scssApp.scss 文件中添加新的样式导入语句,以及对现有样式的修改。build-taro.mjsbuild.mjs 文件的 copyStylesbuildCSS 函数也进行了重构,以简化样式处理和模块构建流程。整体上,这些更改旨在增强样式的模块化和适应性。

Changes

文件路径 更改摘要
packages/nutui-taro-demo/src/app.scss 添加了样式表导入语句 @import '../../../src/styles/jd-font.scss';,并定义了 .demo 类及其修饰符的样式。
scripts/build-taro.mjs 重构 copyStyles 函数以根据项目 ID 导入不同主题文件,更新了 buildCSS 函数以优化 SCSS 文件处理。
scripts/build.mjs 修改 copyStyles 函数以动态调整导入的主题文件,更新了 buildCSS 函数以改善 SCSS 文件的路径解析。
src/sites/mobile/App.scss 添加字体样式表导入,修改了 #app#nav.demo 类的样式,增强了 RTL 布局支持。

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant App
    participant Styles
    User->>App: Load Application
    App->>Styles: Import Stylesheets
    Styles-->>App: Apply Styles
    App-->>User: Render UI
Loading

Possibly related PRs

Suggested labels

size/XL

Suggested reviewers

  • xiaoyatong

兔子欢快地跳跃,
样式更新真不错。
字体新,背景亮,
代码整洁如春光。
让我们一起庆祝吧! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b512259 and 887fd71.

📒 Files selected for processing (2)
  • scripts/build-taro.mjs (1 hunks)
  • scripts/build.mjs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • scripts/build-taro.mjs
  • scripts/build.mjs

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the 3.x label Nov 27, 2024
Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.99%. Comparing base (9332996) to head (887fd71).
Report is 1 commits behind head on feat_v3.x.

Additional details and impacted files
@@            Coverage Diff             @@
##           feat_v3.x    #2798   +/-   ##
==========================================
  Coverage      83.99%   83.99%           
==========================================
  Files            220      220           
  Lines          17885    17885           
  Branches        2679     2679           
==========================================
  Hits           15023    15023           
  Misses          2857     2857           
  Partials           5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (7)
src/sites/mobile/App.scss (4)

Line range hint 1-8: 建议:考虑保留滚动条以提高可访问性

完全隐藏滚动条可能会影响用户体验,特别是对于依赖滚动条进行导航的用户。建议:

  1. 考虑使用细滚动条而不是完全隐藏
  2. 确保键盘导航功能完整可用
html,
body {
  &::-webkit-scrollbar {
-   width: 0;
+   width: 4px;
    background: transparent;
+   &-thumb {
+     background: rgba(0, 0, 0, 0.2);
+     border-radius: 2px;
+   }
  }
}

Line range hint 10-65: 建议:优化导航栏在移动设备上的表现

固定定位的导航栏在某些移动浏览器上可能会出现问题。建议:

  1. 添加 -webkit-overflow-scrolling: touch 以改善 iOS 设备上的滚动体验
  2. 考虑使用 CSS 变量来管理关键尺寸,便于主题定制
  3. 添加安全区域适配
#nav {
  position: fixed;
  z-index: 10;
  left: 0;
  right: 0;
+ padding-top: env(safe-area-inset-top);
  height: 57px;
  line-height: 57px;
  text-align: center;
  background: white;
  font-weight: bold;
  font-size: 20px;
  color: rgba(51, 51, 51, 1);
  box-shadow: 0px 4px 10px 0px rgba(0, 0, 0, 0.07);
+ -webkit-overflow-scrolling: touch;
}

Line range hint 67-116: 建议:改进移动端视口高度处理

使用 vh 单位在移动端可能会导致不一致的行为,特别是在带有动态工具栏的移动浏览器中。建议:

  1. 考虑使用 CSS 变量 var(--vh) 配合 JavaScript 动态设置视口高度
  2. 添加媒体查询以适应不同设备
  3. 优化类名结构,更好地遵循 BEM 命名规范
.demo {
- min-height: 100vh;
+ min-height: calc(var(--vh, 1vh) * 100);
  height: 100%;
  background: #f7f8fa;
  overflow-x: hidden;
  overflow-y: auto;
  padding: 57px 17px 0 17px;
+ @media screen and (max-width: 375px) {
+   padding: 57px 12px 0 12px;
+ }
}

建议添加以下 JavaScript 代码来处理视口高度:

function setVH() {
  document.documentElement.style.setProperty('--vh', `${window.innerHeight * 0.01}px`);
}
window.addEventListener('resize', setVH);
setVH();

Line range hint 118-140: 建议:优化暗色主题实现方式

当前暗色主题实现使用了 !important,这可能会导致样式优先级问题。建议:

  1. 使用 CSS 变量实现主题切换
  2. 避免使用 !important
  3. 考虑添加过渡动画提升用户体验
+ :root {
+   --demo-bg: #f7f8fa;
+ }
+ 
+ .nut-theme-dark {
+   --demo-bg: #000;
+ }

.demo {
-  background: #f7f8fa;
+  background: var(--demo-bg);
+  transition: background-color 0.3s ease;
}

- .nut-theme-dark .demo {
-   background: #000 !important;
- }
scripts/build.mjs (2)

200-203: 主题文件导入顺序需要优化

当前的导入顺序可能会导致样式覆盖问题。建议将默认主题和暗黑主题的导入顺序调整为更合理的方式,以确保样式正确应用。

建议按照以下方式调整导入顺序:

  let content = [
-   `@import '../theme-default.scss';`,
-   `@import '../theme-dark.scss';`,
    `@import '../font-face.scss';`,
+   `@import '../theme-default.scss';`,
+   `@import '../theme-dark.scss';`,
  ]

200-212: 建议增加主题切换的文档说明

当前的主题处理逻辑缺少相应的文档说明,这可能会影响其他开发者的使用。

建议在 README.md 或相关文档中添加以下内容:

  1. 主题文件的命名规范
  2. 如何添加新的项目特定主题
  3. 环境变量 VITE_APP_PROJECT_ID 的使用说明
  4. 主题切换的实现方式

需要我帮您起草这份文档吗?

scripts/build-taro.mjs (1)

240-252: 建议优化主题文件的错误处理

当项目特定主题文件不存在时,当前实现可能会导致构建失败。建议添加错误处理逻辑,在找不到项目特定主题时回退到默认主题。

  let content = [
    `@import '../theme-default.scss';`,
    `@import '../theme-dark.scss';`,
    `@import '../font-face.scss';`,
  ]
  const projectID = process.env.VITE_APP_PROJECT_ID
  if (projectID) {
+   const projectThemePath = resolve(__dirname, `../dist/styles/theme-${projectID}.scss`)
+   try {
+     await access(projectThemePath)
      content = [
        `@import '../theme-${projectID}.scss';`,
        `@import '../font-face.scss';`,
      ]
+   } catch (e) {
+     console.warn(`项目主题文件 theme-${projectID}.scss 不存在,将使用默认主题`)
+   }
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9332996 and 08923e5.

📒 Files selected for processing (4)
  • packages/nutui-taro-demo/src/app.scss (1 hunks)
  • scripts/build-taro.mjs (1 hunks)
  • scripts/build.mjs (1 hunks)
  • src/sites/mobile/App.scss (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/nutui-taro-demo/src/app.scss
🔇 Additional comments (2)
scripts/build.mjs (1)

207-210: ⚠️ Potential issue

验证项目特定主题文件的存在性

当前代码假设项目特定的主题文件一定存在,但如果文件不存在可能会导致构建失败。

建议添加文件存在性检查:

  if (projectID) {
+   const themeFile = resolve(__dirname, `../src/styles/theme-${projectID}.scss`)
+   try {
+     await access(themeFile)
+   } catch (e) {
+     console.error(`主题文件 theme-${projectID}.scss 不存在`)
+     process.exit(1)
+   }
    content = [
      `@import '../theme-${projectID}.scss';`,
      `@import '../font-face.scss';`,
    ]
  }
scripts/build-taro.mjs (1)

240-243: 主题文件导入顺序合理

默认主题、暗黑主题和字体文件的导入顺序设置合理,确保了样式的正确覆盖。

Comment on lines 245 to 250
const projectID = process.env.VITE_APP_PROJECT_ID
if (projectID) {
content[1] = `@import '../variables-${projectID}.scss';`
content = [
`@import '../theme-${projectID}.scss';`,
`@import '../font-face.scss';`,
]
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

需要添加缺失的 font-face.scss 文件

检查结果显示:

  • 项目中存在多个主题文件(theme-dark.scss、theme-default.scss、theme-jmapp.scss、theme-jrkf.scss),且内容结构正确
  • 但是缺少构建脚本中引用的 font-face.scss 文件

建议:

  • 在 src/styles 目录下创建 font-face.scss 文件
  • 或者从构建脚本中移除对 font-face.scss 的引用
🔗 Analysis chain

验证项目特定主题文件的存在性

当设置了 VITE_APP_PROJECT_ID 环境变量时,构建脚本会尝试导入项目特定的主题文件。需要确保这些主题文件在指定位置存在。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证项目特定主题文件是否存在

# 检查主题文件
fd -t f "theme-.*\.scss$" "src/styles"

Length of output: 150


Script:

#!/bin/bash
# 检查主题文件的内容结构,确保它们是有效的 SCSS 文件
for theme in $(fd -t f "theme-.*\.scss$" "src/styles"); do
  echo "=== $theme 内容 ==="
  head -n 5 "$theme"
  echo
done

# 检查 font-face.scss 文件是否存在及其内容
echo "=== font-face.scss 文件检查 ==="
fd -t f "font-face.scss$" "src/styles" -x head -n 5 {}

Length of output: 1380

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
src/styles/jd-font.scss (1)

1-8: 建议优化字体加载策略

以下几点建议供参考:

  1. 当前使用base64编码内嵌字体可能会增加初始CSS文件大小,建议考虑以下方案:

    • 对于较大的字体文件,建议使用单独的字体文件,以便浏览器可以单独缓存
    • 如果必须内嵌,建议只包含最常用的字符子集
  2. 建议添加字体回退机制:

 @font-face {
-  font-family: 'JD';
+  font-family: 'JD', -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif;
   src: url('data:font/ttf;charset=utf-8;base64,...')
     format('truetype');
   font-weight: 400;
   font-style: normal;
   font-display: swap;
 }
  1. 如果项目中需要不同的字重,建议添加对应的字体变体。
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 08923e5 and b512259.

📒 Files selected for processing (1)
  • src/styles/jd-font.scss (1 hunks)
🔇 Additional comments (1)
src/styles/jd-font.scss (1)

3-3: 请验证字体文件的安全性

建议验证base64编码的字体文件来源是否可信,以防止潜在的安全风险。

✅ Verification successful

字体文件来源可信,无需担心安全风险

根据分析,这是京东正版的"京东正黑体"(JDZhengHei V2.0)字体文件。从base64编码的字体数据中可以看到以下信息:

  • 版权所有:北京京东科技有限公司(BEIJING JINGDONG TECHNOLOGY CO., LTD)
  • 字体名称:JDZhengHei V2.0
  • 版本:2.000
  • 字体格式:TrueType (.ttf)

该字体文件是京东官方设计和发布的品牌字体,用于保持品牌一致性,可以安全使用。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查字体文件的完整性和安全性

# 提取并解码base64字体数据
echo "data:font/ttf;charset=utf-8;base64,..." | sed 's/.*base64,//' | base64 -d > font.ttf

# 检查文件类型和完整性
file font.ttf

Length of output: 289


Script:

#!/bin/bash
# 搜索相关的字体文件引用和定义
rg -g '*.{scss,css}' "data:font/ttf;charset=utf-8;base64" -A 2 -B 2

# 搜索相关的字体文件名称
fd -e scss -e css | xargs grep -l "JDZhengHei"

# 查找可能的字体相关配置文件
fd -e json -e yaml -e yml | xargs grep -l "font"

Length of output: 4486

@xiaoyatong xiaoyatong merged commit 15de77e into jdf2e:feat_v3.x Nov 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants