fix(grid): Modify the spacing between the sorting buttons in saas mode. #3603
Conversation
WalkthroughA new "IconTriangleUp" SVG icon component is introduced and integrated across several packages. References to sorting icons in the grid are updated to use the new triangle icons, and the corresponding style for the sort button is adjusted to use a medium text size. The icon is also added to demo icon groups. Additionally, fixed sizing is applied to certain toolbar SVG icons. Changes
Estimated code review effort2 (~15 minutes) 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/demos/pc/app/icon/iconGroups.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. packages/design/saas/src/grid/index.tsOops! 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. packages/vue-icon-saas/index.tsOops! 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.
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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 (
|
![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
🔭 Outside diff range comments (1)
packages/theme-saas/src/grid/table.less (1)
687-693: Font size now exceeds the fixed button box – risk of vertical clipping
@apply text-mdbumps the font-size to 14 px/16 px (depending on Tailwind config) while the element height/line-height are locked at 14 px (~'h-3.5' ; line-height: 14px).
On some zoom levels the glyph will be taller than the box, causing clipping / baseline shift..@{grid-prefix-cls}-sort__btn { @apply ~'h-3.5'; line-height: 14px; @apply block; - @apply text-md; + /* keep visual size while increasing spacing */ + @apply text-xs; /* revert font-size */ + @apply mx-0.5; /* add horizontal gap instead */ @apply fill-color-icon-secondary; transform: scale(0.8); @apply cursor-pointer; }Alternatively, remove the explicit height/line-height and rely on flex/gap for spacing.
Please verify in all supported browsers, especially when the page is zoomed.
🧹 Nitpick comments (2)
examples/sites/demos/pc/app/icon/iconGroups.js (1)
74-76: Group ordering drift
'IconTriangleUp'is appended but the list is roughly alphabetical.
Consider inserting after'IconTriangleDown'to keep predictable ordering and avoid merge-conflicts later.packages/vue-icon-saas/index.ts (1)
383-384: Index file keeps growing – consider auto-generationThe manual addition of
IconTriangleUpworks, but this flat file is already >2 k lines.
Long-term maintainability and tree-shaking suffer.Suggestion: switch to a build-time script that:
- Scans
src/**/index.tsfor default exports.- Generates this barrel module with consistent ordering and both camelCase aliases.
This avoids human error and keeps bundle size in check.
Also applies to: 1458-1460, 2069-2070
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/theme-saas/src/svgs/triangle-down.svgis excluded by!**/*.svgpackages/theme-saas/src/svgs/triangle-up.svgis excluded by!**/*.svgpackages/theme/src/svgs/triangle-down.svgis excluded by!**/*.svgpackages/theme/src/svgs/triangle-up.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
examples/sites/demos/pc/app/icon/iconGroups.js(1 hunks)packages/design/saas/src/grid/index.ts(2 hunks)packages/theme-saas/src/grid/table.less(1 hunks)packages/vue-icon-saas/index.ts(3 hunks)packages/vue-icon/index.ts(3 hunks)packages/vue-icon/src/triangle-up/index.ts(1 hunks)
🧠 Learnings (2)
packages/design/saas/src/grid/index.ts (2)
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中，使用 chart-core 中的 huiChartOption 的组件，不应在其 data 中定义 huiChartOption 或 option，而是应该依赖 chart-core 提供的 huiChartOption。
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:05.740Z
Learning: 在位于packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue的组件中，当使用chart-core时，应删除错误的option定义，使用chart-core中的huiChartOption。
packages/vue-icon/src/triangle-up/index.ts (1)
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中，使用 chart-core 中的 huiChartOption 的组件，不应在其 data 中定义 huiChartOption 或 option，而是应该依赖 chart-core 提供的 huiChartOption。
🧰 Additional context used
🧠 Learnings (2)
packages/design/saas/src/grid/index.ts (2)
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中，使用 chart-core 中的 huiChartOption 的组件，不应在其 data 中定义 huiChartOption 或 option，而是应该依赖 chart-core 提供的 huiChartOption。
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue:30-32
Timestamp: 2024-11-25T03:24:05.740Z
Learning: 在位于packages/vue/src/huicharts/huicharts-sunburst/src/chart-sunburst.vue的组件中，当使用chart-core时，应删除错误的option定义，使用chart-core中的huiChartOption。
packages/vue-icon/src/triangle-up/index.ts (1)
Learnt from: Davont
PR: #2513
File: packages/vue/src/huicharts/huicharts-histogram/src/chart-histogram.vue:33-36
Timestamp: 2024-11-25T03:43:05.285Z
Learning: 在 Tiny Vue 代码库中，使用 chart-core 中的 huiChartOption 的组件，不应在其 data 中定义 huiChartOption 或 option，而是应该依赖 chart-core 提供的 huiChartOption。
🔇 Additional comments (5)
packages/vue-icon/index.ts (3)
383-384: Addition is consistent and alphabetically-placed – no action required.
IconTriangleUpis imported right afterIconTriangleDown, preserving the existing alphabetical order and mirroring the down-triangle pattern.
1457-1460: Good: camelCase alias included.Both the PascalCase export and its
iconTriangleUpalias are declared, matching the convention used for every other icon pair.
2068-2070: Default export list updated correctly.
IconTriangleUpis added to the giant default export object, keeping the icon available for tree-shaking consumers.packages/design/saas/src/grid/index.ts (1)
1-1: Icon swap looks correct – verify build size only.Replacing chevrons with triangles aligns with the new visual spec and keeps the invocation pattern (
IconFoo()) unchanged.
The only follow-up worth checking is the resulting bundle size; if the chevrons are now unused elsewhere, they can eventually be dropped to save a few bytes.Also applies to: 14-15
packages/vue-icon/src/triangle-up/index.ts (1)
12-15: Implementation mirrors existing icon pattern.Uses the common
svghelper with the correct asset and name; nothing else to flag.
PR
fix:修改saas模式表格排序按钮间距
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
New Features
Style