revert: revert mobile first from sync #2980
Conversation
WalkthroughThe changes update CSS class bindings and styling in several Vue component files. In the form-item and numeric mobile-first files, class names are adjusted based on the Changes
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/checkbox/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/input/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. 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 (
|
WalkthroughThis pull request reverts changes related to the mobile-first approach in the Vue components. It primarily adjusts class definitions and styling attributes to remove mobile-first specific styles, ensuring consistency across different display modes. Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| @@ -26,15 +26,15 @@ | |||
| v-if="slots.label || label" | |||
| :class=" | |||
| m( | |||
| 'py-3 sm:py-0 sm:min-h-[theme(spacing.5)] relative align-bottom float-left text-sm pr-3 sm:pr-4 box-border', | |||
| 'py-3 sm:py-0 sm:min-h-[theme(spacing.5)] relative align-bottom float-left text-sm pr-3 sm:pr-4 box-border leading-5', | |||
There was a problem hiding this comment.
Ensure that the removal of mobile-first specific styles does not affect the layout and functionality across different screen sizes. Test thoroughly on various devices.
| @@ -103,7 +103,7 @@ | |||
| <div | |||
| data-tag="numeric-display-only" | |||
| v-if="state.isDisplayOnly" | |||
| class="sm:leading-5.5 text-color-text-primary" | |||
| class="sm:leading-normal text-color-text-primary" | |||
There was a problem hiding this comment.
Verify that the change from sm:leading-5.5 to sm:leading-normal maintains the intended visual hierarchy and readability.
|
[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 |
| @@ -1,5 +1,6 @@ | |||
| export const classes = { | |||
| 'radio-default': 'radio inline-flex items-center leading-4 cursor-pointer sm:flex-row py-px sm:py-0 h-fit', | |||
| 'radio-default': | |||
There was a problem hiding this comment.
Check that the alignment change in radio-default class does not disrupt the layout in different contexts, especially in forms and interactive elements.
![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/theme-saas/src/form-item/index.less (2)
71-74: Adjusted Label Line Height in Medium Variant.
Line 72 introduces@apply ~'leading-5.5';for the&--medium &__labelblock. This refined line height value appears to fine-tune text alignment but should be double-checked for visual consistency with other heading and label styles across components.
151-153: Margin-Top Adjustment for Switch Component in Mini Variant.
In the mini variant (lines 151–153), the switch element now gets@apply ~'mt-0.5';. This reduced top margin should be carefully reviewed to confirm it maintains consistency with the design of other mini components while ensuring proper visual separation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/theme-saas/src/form-item/index.less(1 hunks)packages/vue/src/checkbox/src/mobile-first.vue(1 hunks)packages/vue/src/form-item/src/mobile-first.vue(6 hunks)packages/vue/src/input/src/mobile-first.vue(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/vue/src/input/src/mobile-first.vue
- packages/vue/src/checkbox/src/mobile-first.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue/src/form-item/src/mobile-first.vue
🔇 Additional comments (4)
packages/theme-saas/src/form-item/index.less (4)
4-17: Prefix Variable Definitions Consistency.
All prefix variables (e.g.,@form-item-prefix-cls,@autocomplete-prefix-cls, etc.) are defined using the literal string notation (~'...') which is appropriate for Less when you need to output a literal value. Please confirm that the global variable@css-prefixis defined elsewhere in the project to ensure these expressions resolve correctly.
20-23: Increased Bottom Margin for Form Item Prefix.
Line 21 now appliesmb-5instead of the previousmb-4. This change increases the bottom margin for elements using the form item prefix class, which may affect the vertical spacing in layouts. Please verify that this adjustment aligns with the overall design guidelines and related mobile/desktop styling, especially given the context of reverting mobile-first changes.
103-106: Adjusted Margin-Top for Switch Component in Content Block.
Line 104 applies@apply ~'mt-1.5';to the switch component within the general content section. Ensure that this margin-top value creates harmonious spacing relative to surrounding elements and matches the intended design specifications.
242-248: Changed Margin-Left for Inline Error Display.
Line 247 uses@apply ~'ml-2.5';for the inline error styling. Please verify that this increased left margin provides adequate spacing from adjacent elements without disrupting the overall layout of inline error messages.
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