fix: optimize bind-i18n styles #925
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
github-actions
bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
packages/common/component/I18nInput.vue (1)
144-148: Consider scoping the popover styles.Using global styles (without
scoped) could lead to unintended style conflicts. Consider moving these styles into the scoped section or using a more specific selector.-<style> +<style lang="less" scoped> -.i18n-input-popover { +:deep(.i18n-input-popover) { padding: 20px !important; } </style>packages/common/component/BindI18n.vue (2)
182-183: Consider using CSS custom properties for consistent spacingWhile the styling changes improve layout, consider using CSS custom properties (variables) for consistent spacing values across the component.
<style lang="less" scoped> +:root { + --spacing-base: 8px; + --spacing-medium: 16px; +} .tiny-button.tiny-button.tiny-button--default { margin: 0; } .label { - margin: 16px 0; + margin: var(--spacing-medium) 0; font-size: 12px; line-height: 18px; }Also applies to: 204-210
228-231: Consider scoping the dropdown stylesWhile the max-width constraint is reasonable, consider scoping these styles to avoid potential conflicts with other components using the same class.
-<style> +<style lang="less" scoped> -.select-i18n-drop { +:deep(.select-i18n-drop) { max-width: 300px; } </style>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/common/component/BindI18n.vue(6 hunks)packages/common/component/I18nInput.vue(2 hunks)
🔇 Additional comments (5)
packages/common/component/I18nInput.vue (2)
128-139: LGTM! Well-structured layout.
The positioning and spacing provide a clean, organized layout for the popover content.
7-7: LGTM! Verify the increased popover width.
The increased width and semantic class name improve the UI. However, please verify that the wider popover (340px) doesn't cause layout issues in narrow viewports.
Also applies to: 10-10
✅ Verification successful
Based on the search results, I can see that the application has proper responsive handling with various media queries and width constraints. The I18nInput component's popover width of 340px is well within reasonable limits, considering:
-
The codebase shows several components with similar or larger widths:
- Dialog widths: 700-760px
- Popovers/panels: 300-500px
- Form elements: 300-400px
-
The application has responsive breakpoints at common viewport sizes (1280px, 1400px, 1600px, 1920px) with appropriate styling adjustments.
-
The base minimum width of the application is 1280px (from theme variables), which provides ample space for a 340px popover.
The popover width increase is safe and follows the application's UI patterns.
The wider popover aligns with other similar UI elements in the codebase and won't cause layout issues given the application's responsive design system and minimum viewport width requirements.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any responsive handling or width constraints in the codebase
rg -g '*.{vue,less,css}' '(max-width|min-width|@media|responsive)'
Length of output: 22146
packages/common/component/BindI18n.vue (3)
7-7: LGTM! Improved select component UX
The changes enhance usability through a more descriptive placeholder and controlled dropdown width.
Also applies to: 10-10
28-33: LGTM! Improved button organization and hierarchy
The changes enhance the UI by:
- Using proper button hierarchy (primary/secondary)
- Implementing consistent button spacing
- Improving layout with flex container
48-50: LGTM! Consistent button styling
The changes maintain visual consistency with other button groups in the component.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/common/component/BindI18n.vue (2)
17-29: Consider adding ARIA role for better accessibilityWhile using
div.labelfor block-level layout is justified (per previous discussion), consider adding an ARIA role for better accessibility:-<div class="label">国际化参数配置</div> +<div class="label" role="heading" aria-level="2">国际化参数配置</div>The button layout improvements with flex and proper spacing look good!
Line range hint
178-221: Consider using CSS variables for consistent spacingThe layout improvements look good! Consider extracting common values into CSS variables for better maintainability:
.bottom-buttons { display: flex; justify-content: flex-end; - margin-top: 16px; - gap: 8px; + margin-top: var(--tiny-spacing-4x, 16px); + gap: var(--tiny-spacing-2x, 8px); }This would help maintain consistent spacing across the application.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/common/component/BindI18n.vue(5 hunks)packages/common/component/I18nInput.vue(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/common/component/I18nInput.vue
🔇 Additional comments (3)
packages/common/component/BindI18n.vue (3)
44-46: LGTM! Consistent button styling
The button layout maintains visual consistency with the main section and establishes clear action hierarchy.
64-64: LGTM! Fixed component registration
Properly registering the TinyInput component that was already in use.
7-13: LGTM! Template changes improve UX
The changes enhance usability through:
- More descriptive placeholder text
- Consistent dropdown width with
is-drop-inherit-width - Improved option labels showing both key and translated text
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough context about the is-drop-inherit-width prop usage:
The is-drop-inherit-width prop is correctly implemented and used
The prop is consistently used across the codebase:
- In BindI18n.vue for language selection dropdown
- In SelectConfigurator.vue as a configurable option
- The prop is properly supported by the tiny-select component's CSS implementation as evidenced by the dropdown styling rules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if is-drop-inherit-width prop is documented and consistently used
ast-grep --pattern 'tiny-select $$$is-drop-inherit-width$$$'
Length of output: 62
Script:
#!/bin/bash
# Let's try a different approach to verify the prop usage and documentation
# First, let's search for any documentation or usage of is-drop-inherit-width
rg "is-drop-inherit-width" -A 2 -B 2
# Also search for tiny-select usage patterns to understand the context
rg "tiny-select" -A 2 -B 2
Length of output: 64769
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
优化属性面板绑定国际化UI
What is the current behavior?
Issue Number: N/A
What is the new behavior?
效果图
选择不带参数的国际化文案
选择带参数的国际化文案
添加国际化并关联
Does this PR introduce a breaking change?
Other information
遗留问题：在
tiny-select组件的下拉面板中，固定了面板宽度后，如果某个选项的文本长度超长，会变成可横向滚动的面板（见下图1）。在tiny-vue最新版中，会自动截去多余文本，并且悬停可以显示详情（见下图2）。建议升级tiny-vue版本来解决这个问题，否则需要侵入式修改部分style图1：下拉面板可横向滚动
图2：
tiny-vue最新版选择器Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes