fix: fix form item not align when set display-only #2950
Conversation
WalkthroughThis pull request makes a series of styling updates in several Vue components. Modifications include adjustments to class bindings in mobile-first components for form items, numeric inputs, and switches—primarily controlling spacing, leading, heights, and disabled states based on component state properties. Additionally, token files for numeric and radio components have been updated to modify background, alignment, and cursor styling. No public API declarations were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SwitchComponent
participant State
User->>SwitchComponent: Toggle switch
SwitchComponent->>State: Check state.disabled and types
alt State is disabled or loading
SwitchComponent->>SwitchComponent: Render with "-disabled" classes for overall switch, handle, and icons
else
SwitchComponent->>SwitchComponent: Render with "-not-disabled" (or default) classes
end
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
packages/vue/src/form-item/src/mobile-first.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. packages/vue/src/numeric/src/token.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/src/numeric/src/mobile-first.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.
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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修复了在多端模板中，当表单项设置为仅展示时的对齐问题。通过调整CSS类和条件渲染，确保在不同的显示状态下，表单项能够正确对齐。 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
|
[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 |
![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
🧹 Nitpick comments (2)
packages/vue/src/form-item/src/mobile-first.vue (2)
76-76: Consider extracting magic numbers into CSS variables.The height values (
h-[22px]andh-7) should be extracted into CSS variables for better maintainability and consistency across the component.Consider this refactor:
- state.isDisplayOnly ? '[&_[data-tag=tiny-rate]]:h-[22px]' : '[&_[data-tag=tiny-rate]]:h-7', + state.isDisplayOnly ? `[&_[data-tag=tiny-rate]]:h-[var(--tiny-rate-display-only-height,22px)]` : '[&_[data-tag=tiny-rate]]:h-[var(--tiny-rate-height,theme(spacing.7))]',Also applies to: 80-80
112-113: Consider extracting component-specific styles.The component-specific styles should be extracted into separate classes or CSS variables for better maintainability and reusability.
Consider this refactor:
- ? 'sm:leading-5.5 [&_>*:not([data-tag^=tiny-],[class^=tiny-])]:leading-8 [&_>*:not([data-tag^=tiny-],[class^=tiny-])]:sm:leading-normal' - : '[&_[data-tag=tiny-checkbox]]:h-7 [&_[data-tag=tiny-radio]]:h-7' + ? ` + sm:leading-[var(--tiny-form-item-display-only-leading,1.375rem)] + [&_>*:not([data-tag^=tiny-],[class^=tiny-])]:leading-[var(--tiny-form-item-leading,2rem)] + [&_>*:not([data-tag^=tiny-],[class^=tiny-])]:sm:leading-normal + ` + : ` + [&_[data-tag=tiny-checkbox]]:h-[var(--tiny-checkbox-height,theme(spacing.7))] + [&_[data-tag=tiny-radio]]:h-[var(--tiny-radio-height,theme(spacing.7))] + `
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/vue/src/form-item/src/mobile-first.vue(4 hunks)packages/vue/src/numeric/src/mobile-first.vue(1 hunks)packages/vue/src/numeric/src/token.ts(1 hunks)packages/vue/src/radio/src/token.ts(2 hunks)packages/vue/src/switch/src/mobile-first.vue(4 hunks)
🔇 Additional comments (7)
packages/vue/src/numeric/src/token.ts (1)
28-28: LGTM! Background color addition improves visual consistency.The addition of
bg-color-bg-1class ensures consistent background color for numeric inputs, which helps with the form item alignment in display-only mode.packages/vue/src/radio/src/token.ts (2)
2-2: LGTM! Improved radio button layout.The updated class definition improves the radio button layout with better alignment and spacing.
40-42: LGTM! Enhanced readonly state styling.The changes improve user experience by:
- Setting appropriate cursor style for readonly states
- Maintaining consistent alignment with other form elements
packages/vue/src/switch/src/mobile-first.vue (1)
9-9: LGTM! Improved state management consistency.The changes consistently use
state.disabledinstead of the direct prop, which improves code maintainability and follows Vue's best practices for state management.Also applies to: 28-29, 65-65, 78-78
packages/vue/src/numeric/src/mobile-first.vue (1)
106-106: LGTM! Fixed line height for better alignment.The change from
sm:leading-normaltosm:leading-5.5provides precise line height control, which helps fix the form item alignment in display-only mode.packages/vue/src/form-item/src/mobile-first.vue (2)
29-29: LGTM! Label styling improvements for display-only mode.The changes appropriately simplify label styling in display-only mode by inheriting alignment and adjusting padding, which helps fix the alignment issues.
Also applies to: 37-37
52-52:❓ Verification inconclusive
LGTM! Enhanced content visibility and line height consistency.
The increased max-height and adjusted line heights improve content visibility and maintain consistent spacing. Please verify that these changes don't cause any content clipping issues with longer labels.
Run this script to check for potential content clipping issues:
Also applies to: 56-56
🏁 Script executed:
#!/bin/bash # Description: Search for long label text in the codebase that might be affected by the max-height change. # Search for label prop assignments with long text rg -U "label=[\"\'].*[\"\']" --type vue -A 1 -B 1 # Search for label slot usage with potentially long content rg -U "<template\s+#label>.*</template>" --type vue -A 1 -B 1Length of output: 240
LGTM! Enhanced content visibility and improved spacing.
The updated max-height and line-height adjustments look good overall. However, the previous script intended to check for potential clipping issues with longer label text returned errors. To further verify that no clipping occurs for longer labels, please run the revised script below:
#!/bin/bash # Description: Search for long label text in all Vue files to verify that the max-height change does not lead to content clipping. # Search for label prop assignments with long text in Vue files rg "label=['\"].*['\"]" --glob '*.vue' -A 2 -B 2 # Search for label slot usage with potentially long content in Vue files rg "<template\s+#label>.*</template>" --glob '*.vue' -A 2 -B 2Please confirm that these queries do not expose any clipping issues with long label content.
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