Fix/buttondialog #3309
Fix/buttondialog #3309
Conversation
Walkthrough本次更新为 Button 和 Dialog 组件的 SCSS 样式引入了基于 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Button
participant Dialog
User->>Button: 触发渲染
Button-->>Button: 根据 harmony dynamic 标志应用不同样式
User->>Dialog: 打开对话框
Dialog-->>Dialog: 根据 harmony dynamic 标志应用不同样式
Dialog-->>Dialog: 渲染 close 按钮于新位置
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
github-actions
bot
added
3.x
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/packages/button/button.scss (1)
19-37: 条件编译块产生大量重复属性，后期维护成本高
在/* #ifndef harmony dynamic */与/* #ifdef harmony dynamic */两块中，height / font-size / font-weight / transition / color / background / border-width等属性被硬编码复制。后续若需调整按钮规格，必须在两个分支里同时修改，容易遗漏。建议抽取公共属性或通过 SCSS 变量映射，减少重复。src/packages/dialog/dialog.scss (2)
9-18:box-sizing在文件顶部与动态分支内重复声明
第 9 行和 26 行均写了box-sizing: border-box;，编译后虽无功能性冲突，但冗余会混淆阅读。
86-97: 关闭按钮bottom: -150px魔法数字风险
固定 -150 px 在不同弹窗高度或缩放场景下可能脱离可视区域，无法点击。考虑使用 Flex / 绝对定位相对于父高度的百分比或 CSS 变量。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/button/button.scss(2 hunks)src/packages/dialog/content.tsx(1 hunks)src/packages/dialog/dialog.scss(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: oasis-cloud
PR: jdf2e/nutui-react#2540
File: src/packages/trendarrow/trendarrow.taro.tsx:29-30
Timestamp: 2024-11-06T02:16:07.069Z
Learning: 在`TrendArrow`组件（`src/packages/trendarrow/trendarrow.taro.tsx`）中，CSS变量在鸿蒙系统（Harmony OS）中可能未被完全支持，导致兼容性问题。因此，使用硬编码的十六进制颜色值可以确保在鸿蒙系统上的正确显示。
Learnt from: oasis-cloud
PR: jdf2e/nutui-react#2700
File: src/packages/animatingnumbers/animatingnumbers.harmony.css:25-32
Timestamp: 2024-11-06T05:56:06.800Z
Learning: 在优化 NutUI React 动画性能时，添加 `will-change` 属性可能会对布局产生影响，需要谨慎使用。
src/packages/button/button.scss (2)
Learnt from: oasis-cloud
PR: jdf2e/nutui-react#2700
File: src/packages/animatingnumbers/animatingnumbers.harmony.css:25-32
Timestamp: 2024-11-06T05:56:06.800Z
Learning: 在优化 NutUI React 动画性能时，添加 `will-change` 属性可能会对布局产生影响，需要谨慎使用。
Learnt from: oasis-cloud
PR: jdf2e/nutui-react#2540
File: src/packages/trendarrow/trendarrow.taro.tsx:29-30
Timestamp: 2024-11-06T02:16:07.069Z
Learning: 在`TrendArrow`组件（`src/packages/trendarrow/trendarrow.taro.tsx`）中，CSS变量在鸿蒙系统（Harmony OS）中可能未被完全支持，导致兼容性问题。因此，使用硬编码的十六进制颜色值可以确保在鸿蒙系统上的正确显示。
🧬 Code Graph Analysis (1)
src/packages/dialog/content.tsx (1)
src/packages/dialog/dialog.taro.tsx (1)
close(267-271)
🔇 Additional comments (3)
src/packages/button/button.scss (1)
80-83: 动态模式缺少width: 100%可能导致文字被裁剪
width: 100%仅在非 dynamic 分支里声明。Harmony dynamic 下若父元素宽度不足，.nut-button-wrap可能无法撑满导致内容折行或截断，请确认 UI 需求。src/packages/dialog/dialog.scss (1)
40-53: 外层.nut-dialog-outer动态分支覆盖了原有 Overlay 语义
将宽高设为100%并加深背景rgba(0,0,0,0.8)，可能与 Overlay 组件的遮罩叠加，造成双层遮罩 & 不可滚动。请确认 Overlay 已被移除或相应透明度调整。src/packages/dialog/content.tsx (1)
62-71: 关闭图标移入内部后事件冒泡需要确认
onClick绑定在.nut-dialog-outer，close现在位于内部.nut-dialog。若关闭图标点击事件依赖阻止冒泡以避免触发外层onClick，请确认close组件内部是否正确stopPropagation，否则可能导致点击关闭按钮时同时触发外层回调。
| /* #ifdef harmony dynamic*/ | ||
| background-color: #ffffff; | ||
| width: 80%; | ||
| border-radius: 24px; | ||
| min-width: 240px; | ||
| min-height: 248px; | ||
| padding: 48px; | ||
| box-sizing: border-box; | ||
| position: relative; | ||
| /* #endif */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
动态分支尺寸硬编码，缺乏响应式适配
width: 80%、min-width: 240px、min-height: 248px、padding: 48px 等均为固定像素。不同 DP 值设备或折叠屏上可能出现 UI 失真，建议改用设计系统里的尺寸变量或 vw / rem。
🤖 Prompt for AI Agents
In src/packages/dialog/dialog.scss between lines 19 and 28, the fixed pixel
values for width, min-width, min-height, and padding cause poor responsiveness
on different devices. Replace these hardcoded pixel sizes with relative units
like vw or rem, or use size variables from the design system to ensure the
dialog scales properly across various screen densities and foldable devices.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3309 +/- ##
==========================================
Coverage 88.13% 88.14%
==========================================
Files 290 290
Lines 19110 19110
Branches 2959 2960 +1
==========================================
+ Hits 16843 16844 +1
+ Misses 2262 2261 -1
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| background: $button-default-background-color; | ||
| border-width: $button-border-width; | ||
| /* #endif */ | ||
| /* #ifdef harmony dynamic*/ |
|
cpp 版本中，dialog已适配，本pr关闭。 |
2 participants
1.button组件：
（1）优化鸿蒙环境button组件样式问题，达到官网效果。
处理前：
处理后：
2.dialog组件：
（1）优化鸿蒙环境dialog组件样式问题，达到官网效果。
处理前：
处理后：
（2）处理closeIcon不显示问题。
处理后：
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit