fix(tag): tag add maxWidth prop, and select add maxTagWidth prop #3158
Conversation
WalkthroughThis PR introduces new properties to control the maximum width of tags in both the select and tag components. The updates span demo files, core component files, test scripts, and style sheets for PC and mobile-first environments. New Vue demo components and a Playwright test have been added to showcase and validate the max-width functionality, while CSS utility classes and rules have been updated to handle text overflow and truncation. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as Select Component
participant T as Tag Component
participant CSS as Styling Engine
U->>S: Makes a selection
S->>T: Passes maxTagWidth/max-width property
T->>CSS: Applies inline styles for max width and text overflow
CSS-->>T: Renders tag with controlled width
T-->>S: Displays tag within limits
Possibly related PRs
Suggested labels
Suggested reviewers
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/apis/select.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. examples/sites/demos/apis/tag.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. examples/sites/demos/pc/app/tag/max-width-composition-api.vueOops! 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.
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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:
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 (
|
Walkthrough此PR为 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| import { test, expect } from '@playwright/test' | ||
|
|
||
| test('各型号尺寸是否正常', async ({ page }) => { | ||
| page.on('pageerror', (exception) => expect(exception).not.toBeNull()) |
There was a problem hiding this comment.
Ensure that the page error handler is correctly capturing and logging errors. Consider adding more specific error handling if necessary.
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
WalkthroughThis PR adds the maxWidth property to the tag component and the maxTagWidth property to the select component. These changes include the ability to update related documents and style files to support new properties. Corresponding test cases have also been added to verify these features. Changes
|
![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 (2)
examples/sites/demos/pc/app/tag/max-width.spec.ts (1)
1-10: Test name could be more specific to the maxWidth featureWhile the test correctly verifies that the tag respects the specified maxWidth of 80px, the test name "各型号尺寸是否正常" (roughly "whether different model sizes are normal") doesn't specifically indicate that it's testing the maxWidth feature. Consider renaming it to something more specific like "标签的maxWidth属性是否生效" (whether the tag's maxWidth property takes effect).
Otherwise, the test implementation looks good.
-test('各型号尺寸是否正常', async ({ page }) => { +test('标签的maxWidth属性是否生效', async ({ page }) => {packages/vue/src/tag/src/mobile-first.vue (1)
54-59: Consider adding unit validation for maxWidth.The implementation for maxWidth looks good, but there's no validation to ensure the value includes a valid CSS unit (px, %, em, etc.). If a number without a unit is provided, it might not work as expected in some browsers.
if (maxWidth) { - styles.maxWidth = maxWidth + styles.maxWidth = typeof maxWidth === 'number' ? `${maxWidth}px` : maxWidth styles.display = 'inline-block' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
examples/sites/demos/apis/select.js(1 hunks)examples/sites/demos/apis/tag.js(1 hunks)examples/sites/demos/pc/app/tag/max-width-composition-api.vue(1 hunks)examples/sites/demos/pc/app/tag/max-width.spec.ts(1 hunks)examples/sites/demos/pc/app/tag/max-width.vue(1 hunks)examples/sites/demos/pc/app/tag/webdoc/tag.js(1 hunks)packages/theme-saas/src/tag/index.less(1 hunks)packages/theme/src/tag/index.less(1 hunks)packages/vue/src/select/src/index.ts(1 hunks)packages/vue/src/select/src/mobile-first.vue(6 hunks)packages/vue/src/select/src/pc.vue(6 hunks)packages/vue/src/tag/src/index.ts(1 hunks)packages/vue/src/tag/src/mobile-first.vue(2 hunks)packages/vue/src/tag/src/pc.vue(3 hunks)
🔇 Additional comments (26)
packages/vue/src/tag/src/pc.vue (3)
35-36: LGTM:maxWidthprop added to tag componentThe new
maxWidthprop has been properly added to the component's props array.
55-56: LGTM: Proper destructuring of the maxWidth propThe maxWidth prop is correctly destructured from the component instance along with other properties.
81-84: LGTM: Style implementation for maxWidthThe implementation sets both maxWidth and display properties when maxWidth is provided. Setting display to 'inline-block' is necessary for text truncation to work properly with the CSS classes added in the theme file.
examples/sites/demos/pc/app/tag/max-width.vue (3)
1-5: LGTM: Well-structured demo templateThe demo clearly showcases the new maxWidth feature with a practical example using a long text string that would trigger truncation.
7-15: LGTM: Clean component implementationThe component imports and registers TinyTag properly, following the project's conventions.
17-22: LGTM: Appropriate styling for the demoThe margins provide good spacing for the demo display.
packages/theme-saas/src/tag/index.less (1)
18-20: LGTM: Essential CSS utilities added for text truncationThese two CSS utilities work together to ensure text is properly truncated with an ellipsis when the maxWidth property is applied:
overflow-hiddenprevents text from spilling outside the containertext-ellipsisadds the ellipsis when text is truncatedThis is essential for the maxWidth property to function correctly.
packages/vue/src/select/src/index.ts (1)
366-370: Well-structured addition of themaxTagWidthproperty.This new property for configuring tag width in multi-select scenarios is well-documented and follows the component's existing prop pattern. The type definition correctly allows both String and Number values with an appropriate default.
packages/vue/src/tag/src/index.ts (1)
42-45: Appropriate implementation of themaxWidthproperty.The
maxWidthproperty is correctly defined to allow both String and Number types with a null default, consistent with the component's existing property definitions.packages/vue/src/select/src/pc.vue (6)
89-89: Consistent application ofmaxTagWidthprop to the first tag.The first occurrence of the
maxTagWidthprop is correctly added to the tag that displays the first selected item.
123-123: Proper implementation ofmaxTagWidthfor the counter tag.The
maxTagWidthprop is correctly added to the tag that displays the count of additional selected items.
140-140: Appropriate application ofmaxTagWidthto the "all" text tag.The
maxTagWidthprop is correctly added to the tag that displays the "all" text whenshowAllTextTagis true.
156-156: Consistent implementation ofmaxTagWidthfor the collapse tag.The
maxTagWidthprop is correctly added to the collapse tag that appears when using hover expand functionality.
175-175: Thorough implementation ofmaxTagWidthfor individual selected item tags.The
maxTagWidthprop is correctly added to the tags that display each selected item in the loop.
770-771: Complete implementation with prop registration.The
maxTagWidthproperty is correctly added to the component's props array, maintaining the component structure.examples/sites/demos/pc/app/tag/max-width-composition-api.vue (1)
1-16: Well-structured demo for the newmaxWidthfeature.The demo clearly illustrates the use of the
maxWidthproperty with a realistic example of text that would overflow without this constraint. The component is correctly imported and the styling is consistent with other demos.examples/sites/demos/pc/app/tag/webdoc/tag.js (1)
79-90: LGTM! Properly documented new feature.The addition of the 'max-width' demo is well-structured with appropriate names and descriptions in both Chinese and English, properly showcasing the new feature.
examples/sites/demos/apis/tag.js (1)
57-70: LGTM! API documentation is clear and complete.The new 'max-width' property is well-documented with appropriate type, default value, descriptions, and version information. Support for both PC and mobile-first environments is properly indicated.
packages/vue/src/tag/src/mobile-first.vue (2)
27-28: LGTM! New prop added correctly.The 'maxWidth' property has been properly added to the props array.
62-62: LGTM! Style application is correct.The style binding has been properly updated to use the styles object that includes the maxWidth property when specified.
examples/sites/demos/apis/select.js (1)
547-560: LGTM - New property added to support maximum tag width controlThe implementation of the
max-tag-widthproperty is well-documented with clear descriptions in both Chinese and English. The property is correctly configured to work in both 'pc' and 'mobile-first' modes with appropriate versioning metadata.I like that this addition will allow developers to control tag widths in multi-select scenarios, improving UI consistency and preventing layout issues from overly wide tags.
packages/vue/src/select/src/mobile-first.vue (5)
90-90: LGTM - Properly implemented maxWidth for the first tag instanceThe
maxTagWidthproperty is correctly bound to themaxWidthattribute on the first tag instance, allowing for width control when the tag is rendered.
124-124: LGTM - Properly implemented maxWidth for count tagThe
maxTagWidthproperty is correctly applied to the tag that shows the "+N" count when there are multiple selections, ensuring consistent styling across all tag elements.
144-144: LGTM - Properly implemented maxWidth for collapse tagThe
maxTagWidthproperty is properly applied to the tag that displays when tags are collapsed, ensuring consistent width constraints.
157-157: LGTM - Properly implemented maxWidth for individual selection tagsThe
maxTagWidthproperty is correctly applied to each individual tag in the selection list, ensuring that all tags have consistent width constraints regardless of their content.
718-719: LGTM - Property added to component props arrayThe
maxTagWidthproperty is correctly added to the component's props array, making it accessible throughout the component.
| text-overflow: ellipsis; | ||
| overflow: hidden; | ||
| white-space: nowrap; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CSS properties are duplicated.
The CSS properties for text truncation (text-overflow: ellipsis;, overflow: hidden;, and white-space: nowrap;) are added at lines 21-23, but these same properties already exist at lines 32-34. This duplication is unnecessary and could lead to maintenance issues.
- text-overflow: ellipsis;
- overflow: hidden;
- white-space: nowrap;📝 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.
| text-overflow: ellipsis; | |
| overflow: hidden; | |
| white-space: nowrap; |
PR
为tag添加 maxWidth 属性， 为select 添加 maxTagWidth属性
添加相应的文档
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