feat(tag): [tag] add round prop to support circular border radius #3846
Conversation
WalkthroughAdds a boolean Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
examples/sites/demos/pc/app/tag/round.vue (1)
5-14: Consider simplifying boolean prop syntax.The explicit
:round="true"and:only-icon="true"syntax works correctly but can be simplified. In Vue, boolean props can be set using shorthand syntax.Apply this diff for cleaner code:
- <tiny-tag type="primary" :round="true">圆角标签</tiny-tag> - <tiny-tag type="success" :round="true">成功标签</tiny-tag> - <tiny-tag type="warning" :round="true">警告标签</tiny-tag> - <tiny-tag type="danger" :round="true">危险标签</tiny-tag> + <tiny-tag type="primary" round>圆角标签</tiny-tag> + <tiny-tag type="success" round>成功标签</tiny-tag> + <tiny-tag type="warning" round>警告标签</tiny-tag> + <tiny-tag type="danger" round>危险标签</tiny-tag> - <tiny-tag type="info" :round="true" :only-icon="true"> + <tiny-tag type="info" round only-icon> <template #default> <icon-heartempty /> </template>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/sites/demos/apis/tag.js(1 hunks)examples/sites/demos/pc/app/tag/round-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tag/round.spec.ts(1 hunks)examples/sites/demos/pc/app/tag/round.vue(1 hunks)examples/sites/demos/pc/app/tag/webdoc/tag.js(1 hunks)packages/vue/src/tag/src/index.ts(1 hunks)packages/vue/src/tag/src/pc.vue(4 hunks)
⏰ 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)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (6)
packages/vue/src/tag/src/index.ts (1)
46-49: LGTM! Prop definition follows established patterns.The
roundprop is correctly defined as a Boolean with a sensible default value offalse, consistent with other boolean props in the component.packages/vue/src/tag/src/pc.vue (4)
36-37: LGTM! Prop added to component declaration.The
roundprop is correctly added to the component's props array.
57-58: LGTM! Prop correctly destructured.The
roundprop is properly extracted from the component instance for use in the render function.
70-71: LGTM! CSS class conditionally applied.The
tiny-tag--roundclass is correctly applied when theroundprop is truthy, following the existing pattern for other conditional classes.
88-90: LGTM! Border radius implementation is correct.Setting
borderRadius: '9999px'is a standard technique for creating fully rounded (pill-shaped) elements, as the large value ensures the ends are circular regardless of the element's height.examples/sites/demos/pc/app/tag/webdoc/tag.js (1)
117-140: LGTM! Demo entry properly structured.The webdoc entry for the
roundfeature is correctly structured and consistent with other demo entries in the file.
| desc: { | ||
| 'zh-CN': '是否为圆角的模式', | ||
| 'en-US': 'Whether it is a circular mode' | ||
| }, |
There was a problem hiding this comment.
Clarify the English description.
The English description "Whether it is a circular mode" is ambiguous. "Circular" suggests a circle shape, but round here refers to rounded corners (border-radius). The Chinese "圆角" correctly translates to "rounded corners."
Apply this diff to improve clarity:
desc: {
'zh-CN': '是否为圆角的模式',
- 'en-US': 'Whether it is a circular mode'
+ 'en-US': 'Whether to enable rounded corners'
},📝 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.
| desc: { | |
| 'zh-CN': '是否为圆角的模式', | |
| 'en-US': 'Whether it is a circular mode' | |
| }, | |
| desc: { | |
| 'zh-CN': '是否为圆角的模式', | |
| 'en-US': 'Whether to enable rounded corners' | |
| }, |
🤖 Prompt for AI Agents
In examples/sites/demos/apis/tag.js around lines 183 to 186, the English
description "Whether it is a circular mode" is ambiguous; replace it with a
clear phrase that matches the Chinese "圆角" meaning rounded corners (e.g.,
"Whether it has rounded corners" or "Enable rounded corners"). Update the
'en-US' value to the chosen clear phrasing and keep the existing 'zh-CN' value
unchanged.
| <script setup> | ||
| import { TinyTag } from '@opentiny/vue' | ||
| import { IconHeartempty } from '@opentiny/vue-icon' | ||
| const tinyIconHeartempty = IconHeartempty() | ||
| </script> |
There was a problem hiding this comment.
Remove unused variable or register the component properly.
The variable tinyIconHeartempty is initialized but never used. The template directly references <tiny-icon-heartempty /> without explicit component registration, which may rely on auto-registration behavior.
Consider one of these solutions:
Option 1: Remove the unused variable and import (if auto-registration works):
<script setup>
import { TinyTag } from '@opentiny/vue'
-import { IconHeartempty } from '@opentiny/vue-icon'
-
-const tinyIconHeartempty = IconHeartempty()
</script>Option 2: Register and use the component explicitly:
<script setup>
import { TinyTag } from '@opentiny/vue'
import { IconHeartempty } from '@opentiny/vue-icon'
-const tinyIconHeartempty = IconHeartempty()
+const TinyIconHeartempty = IconHeartempty()
</script>📝 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.
| <script setup> | |
| import { TinyTag } from '@opentiny/vue' | |
| import { IconHeartempty } from '@opentiny/vue-icon' | |
| const tinyIconHeartempty = IconHeartempty() | |
| </script> | |
| <script setup> | |
| import { TinyTag } from '@opentiny/vue' | |
| </script> |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/tag/round-composition-api.vue around lines 16 to
21, the variable tinyIconHeartempty is created but never used while the template
expects a <tiny-icon-heartempty /> component; either remove the unused
IconHeartempty import and tinyIconHeartempty constant (if relying on
auto-registration) or explicitly register the component and use the variable
(e.g., register IconHeartempty as tiny-icon-heartempty in the component context
or expose tinyIconHeartempty for the template) so the template reference matches
a registered component.
| test('round 属性样式验证', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).not.toBeNull()) | ||
| await page.goto('tag#round-usage') | ||
|
|
||
| // 验证所有具有round属性的标签都具有正确的圆角样式 | ||
| const roundTags = page.locator('.tiny-tag') | ||
| const count = await roundTags.count() | ||
|
|
||
| for (let i = 0; i < count; i++) { | ||
| const tag = roundTags.nth(i) | ||
| await expect(tag).toHaveCSS('border-radius', '9999px') | ||
| } | ||
|
|
||
| // 验证标签数量是否正确 | ||
| expect(count).toBe(6) // 4个基础标签 + 1个图标标签 + 1个配置标签 | ||
| }) |
There was a problem hiding this comment.
Test count expectation is incorrect.
Line 55 expects 6 tags, but the demos (round.vue and round-composition-api.vue) only contain 5 tags:
- 4 basic tags (圆角标签, 成功标签, 警告标签, 危险标签)
- 1 icon-only tag
The comment references "1个配置标签" (1 config tag) that doesn't exist.
Update the expected count or add the missing tag:
- // 验证标签数量是否正确
- expect(count).toBe(6) // 4个基础标签 + 1个图标标签 + 1个配置标签
+ // 验证标签数量是否正确
+ expect(count).toBe(5) // 4个基础标签 + 1个图标标签📝 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.
| test('round 属性样式验证', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).not.toBeNull()) | |
| await page.goto('tag#round-usage') | |
| // 验证所有具有round属性的标签都具有正确的圆角样式 | |
| const roundTags = page.locator('.tiny-tag') | |
| const count = await roundTags.count() | |
| for (let i = 0; i < count; i++) { | |
| const tag = roundTags.nth(i) | |
| await expect(tag).toHaveCSS('border-radius', '9999px') | |
| } | |
| // 验证标签数量是否正确 | |
| expect(count).toBe(6) // 4个基础标签 + 1个图标标签 + 1个配置标签 | |
| }) | |
| test('round 属性样式验证', async ({ page }) => { | |
| page.on('pageerror', (exception) => expect(exception).not.toBeNull()) | |
| await page.goto('tag#round-usage') | |
| // 验证所有具有round属性的标签都具有正确的圆角样式 | |
| const roundTags = page.locator('.tiny-tag') | |
| const count = await roundTags.count() | |
| for (let i = 0; i < count; i++) { | |
| const tag = roundTags.nth(i) | |
| await expect(tag).toHaveCSS('border-radius', '9999px') | |
| } | |
| // 验证标签数量是否正确 | |
| expect(count).toBe(5) // 4个基础标签 + 1个图标标签 | |
| }) |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/tag/round.spec.ts around lines 41 to 56, the test
asserts expect(count).toBe(6) but the demos only render 5 tags; update the test
to match the source of truth by changing the expectation to
expect(count).toBe(5) (or alternatively add the missing config tag to the round
demo files if the intention is to keep 6), and run the spec to verify it passes.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/sites/demos/pc/app/tag/round.spec.ts(1 hunks)
⏰ 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)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (1)
examples/sites/demos/pc/app/tag/round.spec.ts (1)
7-8: LGTM!The locator chain correctly targets tags within the demo container, and getting the count enables the dynamic assertion pattern.
| import { test, expect } from '@playwright/test' | ||
|
|
||
| test('圆角', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).toBeNull()) |
There was a problem hiding this comment.
Improve error handling for better debugging.
Using expect(exception).toBeNull() will fail the test but hide the actual error details. Consider logging the exception or letting it propagate naturally for clearer failure messages.
Apply this diff to improve error visibility:
- page.on('pageerror', (exception) => expect(exception).toBeNull())
+ page.on('pageerror', (exception) => {
+ throw new Error(`Unexpected page error: ${exception}`)
+ })Or simply remove the handler to let errors fail the test naturally with full stack traces.
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/tag/round.spec.ts around line 4 the
page.on('pageerror', (exception) => expect(exception).toBeNull()) handler
swallows useful error details by asserting null; replace it with either removing
the handler so errors propagate naturally, or change it to log/throw the
exception (e.g., console.error(exception) and rethrow or call
fail(String(exception))) so test failures include the actual error message and
stack trace for debugging.
| // 动态创建与元素数量相同的期望数组 | ||
| await expect(tags).toHaveClass(Array(count).fill(/tiny-tag--round/)) |
There was a problem hiding this comment.
Test will pass with zero tags, losing validation effectiveness.
The dynamic assertion Array(count).fill(/tiny-tag--round/) creates an empty array when count = 0, causing toHaveClass([]) to pass without validating anything. Additionally, the previous version verified the actual border-radius CSS property, which is no longer checked.
Apply this diff to add meaningful validation:
const tags = page.locator('.all-demos-container').locator('.tiny-tag')
const count = await tags.count()
+ // Ensure at least one round tag exists
+ expect(count).toBeGreaterThan(0)
+
// 动态创建与元素数量相同的期望数组
await expect(tags).toHaveClass(Array(count).fill(/tiny-tag--round/))
+
+ // Verify the first tag has the expected border-radius
+ await expect(tags.first()).toHaveCSS('border-radius', '9999px')This ensures:
- At least one tag is present
- All tags have the
tiny-tag--roundclass - The border-radius CSS property is actually applied
📝 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.
| // 动态创建与元素数量相同的期望数组 | |
| await expect(tags).toHaveClass(Array(count).fill(/tiny-tag--round/)) | |
| const tags = page.locator('.all-demos-container').locator('.tiny-tag') | |
| const count = await tags.count() | |
| // Ensure at least one round tag exists | |
| expect(count).toBeGreaterThan(0) | |
| // 动态创建与元素数量相同的期望数组 | |
| await expect(tags).toHaveClass(Array(count).fill(/tiny-tag--round/)) | |
| // Verify the first tag has the expected border-radius | |
| await expect(tags.first()).toHaveCSS('border-radius', '9999px') |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/tag/round.spec.ts around lines 10-11, the test
currently uses Array(count).fill(/tiny-tag--round/) which passes when count is 0
and no CSS is validated; change the assertions to first assert that tags.length
is greater than 0, then assert that every tag element has the "tiny-tag--round"
class (not just an empty-array class check), and finally check the computed
style on a tag (e.g., getComputedStyle or equivalent) to verify the
border-radius value matches the expected rounded value; implement these three
checks in place of the current single toHaveClass line.
| 'zh-CN': '是否为圆角的模式', | ||
| 'en-US': 'Whether it is a circular mode' | ||
| }, | ||
| mode: ['pc'], |
| export default { | ||
| components: { | ||
| TinyTag: Tag, | ||
| IconHeartempty: Icon.IconHeartempty |
|
@IKEYCY 有两个检视意见，有空可以看下哈 |
@IKEYCY has two review opinions. You can take a look when you have time. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/sites/demos/pc/app/tag/round.vue (1)
18-18: Icon usage pattern requires verification (duplicate concern).A past review comment flagged the icon usage method as problematic. The current code still registers the icon using
IconHeartempty()with a function call. Please verify the correct usage pattern for@opentiny/vue-icon.@opentiny/vue-icon correct usage pattern for IconHeartempty component registrationAlso applies to: 23-23
🧹 Nitpick comments (1)
examples/sites/demos/pc/app/tag/round.vue (1)
5-10: Consider showing both round and default states for comparison.The demo effectively showcases the round prop, but adding tags without
round(or:round="false") would help users visually compare the difference between rounded and default border-radius styles.💡 Example enhancement
<template> <div class="tag-round-demo"> <h4>Round 属性示例</h4> + + <h5>默认标签</h5> + <tiny-tag type="primary">默认标签</tiny-tag> + <tiny-tag type="success">成功标签</tiny-tag> + <tiny-tag type="warning">警告标签</tiny-tag> + <tiny-tag type="danger">危险标签</tiny-tag> + + <h5>圆角标签</h5> <tiny-tag type="primary" :round="true">圆角标签</tiny-tag> <tiny-tag type="success" :round="true">成功标签</tiny-tag>
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/sites/demos/pc/app/tag/round.vue
⏰ 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)
- GitHub Check: PR E2E Test (pnpm test:e2e3)
| <tiny-tag type="info" :round="true" :only-icon="true"> | ||
| <tiny-icon-heartempty /> | ||
| </tiny-tag> |
There was a problem hiding this comment.
Add accessible label for icon-only tag.
The icon-only tag lacks an accessible label for screen reader users. Consider adding an aria-label or title attribute to improve accessibility.
🔎 Suggested fix
- <tiny-tag type="info" :round="true" :only-icon="true">
+ <tiny-tag type="info" :round="true" :only-icon="true" aria-label="收藏">
<tiny-icon-heartempty />
</tiny-tag>📝 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.
| <tiny-tag type="info" :round="true" :only-icon="true"> | |
| <tiny-icon-heartempty /> | |
| </tiny-tag> | |
| <tiny-tag type="info" :round="true" :only-icon="true" aria-label="收藏"> | |
| <tiny-icon-heartempty /> | |
| </tiny-tag> |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/tag/round.vue around lines 10 to 12, the
icon-only <tiny-tag> lacks an accessible label for screen readers; add an
aria-label (or title) to the <tiny-tag> (e.g., aria-label="favorite" or a
relevant description) so the icon-only control is announced, ensuring the
attribute is present when :only-icon is true and matches the icon semantics.
4 participants
PR
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
Documentation / Demos
Tests
✏️ Tip: You can customize this high-level summary in your review settings.