Conversation
WalkthroughChanges update link component styling through margin adjustments in demo files, introduce a new responsive link stylesheet for 1280px max-width breakpoint, switch the theme import source from adaptation to responsive styling, and add a button-group adapt variable for icon radius. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/theme/src/link/adaptation.less (1)
17-19: Prefer a theme token instead of hardcoding success color.Using a CSS variable here keeps adaptation styles consistent with theming/customization.
Proposed refactor
&&--success { - color: `#398500`; + color: var(--tv-Link-success-text-color, `#398500`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/theme/src/link/adaptation.less` around lines 17 - 19, Replace the hardcoded hex color in the adaptation selector "&&--success" with the theme token variable used by the project (e.g. the global CSS custom property for success color) so adaptation styles follow theming; locate the "&&--success" rule in adaptation.less and change its color from "#398500" to the project success token (for example var(--color-success) or the relevant token name used elsewhere) ensuring the variable exists and falls back if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/theme/src/link/adaptation.less`:
- Around line 13-15: Update the CSS block that currently sets display:
inline-flex; white-space: nowrap; line-height: 20px in adaptation.less so long
localized link text is safely truncated on small screens: keep display:
inline-flex but add min-width: 0; replace the unconstrained white-space behavior
by adding overflow: hidden; text-overflow: ellipsis; and keep white-space:
nowrap (or remove it if you prefer wrapping) so the link respects its flex
container and shows an ellipsis instead of overflowing; ensure the rule still
includes line-height: 20px.
---
Nitpick comments:
In `@packages/theme/src/link/adaptation.less`:
- Around line 17-19: Replace the hardcoded hex color in the adaptation selector
"&&--success" with the theme token variable used by the project (e.g. the global
CSS custom property for success color) so adaptation styles follow theming;
locate the "&&--success" rule in adaptation.less and change its color from
"#398500" to the project success token (for example var(--color-success) or the
relevant token name used elsewhere) ensuring the variable exists and falls back
if necessary.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/sites/demos/pc/app/link/custom-icon-composition-api.vueexamples/sites/demos/pc/app/link/custom-icon.vuepackages/theme/src/adaptation-index.lesspackages/theme/src/button-group/vars.lesspackages/theme/src/link/adaptation.less
| display: inline-flex; | ||
| white-space: nowrap; | ||
| line-height: 20px; |
There was a problem hiding this comment.
nowrap here can break small-screen layout with long link text.
Without truncation constraints, long localized labels can overflow horizontally at the exact breakpoint this file targets.
Proposed fix
.@{link-prefix-cls} {
.inject-Link-vars();
display: inline-flex;
+ max-width: 100%;
+ min-width: 0;
white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
line-height: 20px;📝 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.
| display: inline-flex; | |
| white-space: nowrap; | |
| line-height: 20px; | |
| display: inline-flex; | |
| max-width: 100%; | |
| min-width: 0; | |
| white-space: nowrap; | |
| overflow: hidden; | |
| text-overflow: ellipsis; | |
| line-height: 20px; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/theme/src/link/adaptation.less` around lines 13 - 15, Update the CSS
block that currently sets display: inline-flex; white-space: nowrap;
line-height: 20px in adaptation.less so long localized link text is safely
truncated on small screens: keep display: inline-flex but add min-width: 0;
replace the unconstrained white-space behavior by adding overflow: hidden;
text-overflow: ellipsis; and keep white-space: nowrap (or remove it if you
prefer wrapping) so the link respects its flex container and shows an ellipsis
instead of overflowing; ensure the rule still includes line-height: 20px.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/theme/src/link/adaptation.less (1)
6-7: Unused variable declarations.
@link-svg-prefix-clsand@svg-clsare declared but never used in this file. If these are intended for future use, consider removing them until needed to keep the file clean.♻️ Proposed cleanup
`@link-prefix-cls`: ~'@{css-prefix}link'; -@link-svg-prefix-cls: ~'@{link-prefix-cls}-svg'; -@svg-cls: ~'@{css-prefix}svg';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/theme/src/link/adaptation.less` around lines 6 - 7, Remove the unused Less variables `@link-svg-prefix-cls` and `@svg-cls` from the adaptation.less file (or use them if they were intended): locate the declarations of `@link-svg-prefix-cls` and `@svg-cls` and either delete those two lines or replace their usage where appropriate (e.g., in selectors or mixins that reference @{link-prefix-cls}-svg or @{css-prefix}svg) so the file no longer contains dead variable declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/theme/src/link/adaptation.less`:
- Around line 17-19: Replace the hardcoded success color in the link selector
"&&--success" with the theme variable: change the color value from "#398500" to
use "var(--tv-Link-success-text-color)" so the link component uses the
centralized success text color defined in vars.less.
---
Nitpick comments:
In `@packages/theme/src/link/adaptation.less`:
- Around line 6-7: Remove the unused Less variables `@link-svg-prefix-cls` and
`@svg-cls` from the adaptation.less file (or use them if they were intended):
locate the declarations of `@link-svg-prefix-cls` and `@svg-cls` and either delete
those two lines or replace their usage where appropriate (e.g., in selectors or
mixins that reference @{link-prefix-cls}-svg or @{css-prefix}svg) so the file no
longer contains dead variable declarations.
| &&--success { | ||
| color: #398500; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for success color variables in the theme
rg -n '#398500|color-success|success-color' packages/theme/src --type less -C2Repository: opentiny/tiny-vue
Length of output: 43877
🏁 Script executed:
#!/bin/bash
# Explore the theme directory structure to understand color variable organization
find packages/theme/src -name "*.less" -type f | head -20Repository: opentiny/tiny-vue
Length of output: 834
🏁 Script executed:
#!/bin/bash
# Search for color variable definitions more broadly
rg -n '@.*success.*color|@.*color.*success' packages/theme/src --type less -A1 -B1Repository: opentiny/tiny-vue
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check the vars.less or similar files for color definitions
find packages/theme/src -name "*var*" -o -name "*color*" | head -20Repository: opentiny/tiny-vue
Length of output: 846
🏁 Script executed:
#!/bin/bash
# Look at the adaptation.less file to see how other colors are defined
cat -n packages/theme/src/link/adaptation.less | head -40Repository: opentiny/tiny-vue
Length of output: 615
Use the link success text color variable instead of the hardcoded color.
Replace #398500 with var(--tv-Link-success-text-color) to maintain consistency with the theme variables already defined in vars.less. The standard success text color variable is already available and used elsewhere in the link component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/theme/src/link/adaptation.less` around lines 17 - 19, Replace the
hardcoded success color in the link selector "&&--success" with the theme
variable: change the color value from "#398500" to use
"var(--tv-Link-success-text-color)" so the link component uses the centralized
success text color defined in vars.less.
| line-height: 20px; | ||
|
|
||
| &&--success { | ||
| color: #398500; |
There was a problem hiding this comment.
设计师给的颜色值应该是需要在我们的基础变量中的,如果不在需要和设计师交流好,为啥之前的基础变量没有这个颜色
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/theme/src/link/responsive.less (1)
5-7: Unused variable declarations.
@link-svg-prefix-clsand@svg-clsare declared but never referenced within this file. If they're not needed for the responsive link styles, consider removing them to keep the file clean.♻️ Proposed cleanup
`@link-prefix-cls`: ~'@{css-prefix}link'; -@link-svg-prefix-cls: ~'@{link-prefix-cls}-svg'; -@svg-cls: ~'@{css-prefix}svg';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/theme/src/link/responsive.less` around lines 5 - 7, The file declares unused Less variables `@link-svg-prefix-cls` and `@svg-cls` which should be removed to clean up responsive.less; locate the declarations of `@link-svg-prefix-cls` and `@svg-cls` near `@link-prefix-cls` and delete them (or, if they are intended to be used, reference them in the responsive link styles instead), ensuring only `@link-prefix-cls` remains if SVG prefixes are not needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/theme/src/link/responsive.less`:
- Around line 5-7: The file declares unused Less variables `@link-svg-prefix-cls`
and `@svg-cls` which should be removed to clean up responsive.less; locate the
declarations of `@link-svg-prefix-cls` and `@svg-cls` near `@link-prefix-cls` and
delete them (or, if they are intended to be used, reference them in the
responsive link styles instead), ensuring only `@link-prefix-cls` remains if SVG
prefixes are not needed.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/theme/src/adaptation-index.lesspackages/theme/src/link/responsive.less
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/theme/src/adaptation-index.less
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
适配link组件小屏显示
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
Release Notes
Style
Chores