fix: Modify UI review comments #916
Conversation
WalkthroughThis pull request introduces a series of updates across multiple Vue components, primarily focusing on user interface enhancements and styling adjustments. Key changes include modifications to label widths, the addition of new classes for styling, and updates to button structures for improved visual consistency. The logic and functionality of the components largely remain unchanged, with a focus on refining the user experience through visual improvements and better interaction feedback. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (13)
packages/plugins/state/src/CreateVariable.vue (2)
56-56: LGTM! Consider adding accessibility attributes.The removal of redundant
<code>tags while keeping<pre>tags is a good simplification. However, for better accessibility, consider adding appropriate ARIA roles and language attributes.-<pre>{{ getterExample }}</pre> +<pre role="code" aria-label="Getter example" lang="javascript">{{ getterExample }}</pre>Also applies to: 62-62, 75-75, 81-81
408-417: LGTM! Consider extracting common code styles.The styling updates improve readability and maintain consistency with the design system through CSS variables. The monospace font family is appropriate for code examples.
Consider extracting the code-specific styles into a reusable class since these styles might be needed in other components that display code examples:
+.code-block { + font-family: Consolas, 'Courier New', monospace; + background: var(--te-common-bg-container); + color: var(--te-common-text-weaken); + border-radius: 4px; + padding: 8px 14px; +} .tips { font-size: 12px; line-height: 18px; margin-top: 8px; - border-radius: 4px; - padding: 8px 14px; - background: var(--te-common-bg-container); - color: var(--te-common-text-weaken); + @extend .code-block; & > pre { - font-family: Consolas, 'Courier New', monospace; + // Inherit font-family from parent } }packages/plugins/materials/src/meta/layout/src/Main.vue (2)
107-115: LGTM! Consider extracting common styles.The border radius styling for tabs looks good and follows design system conventions. However, the repeated border radius value could be extracted into a common class for better maintainability.
Consider refactoring to reduce repetition:
+:deep(.tiny-tabs__item--rounded) { + border-radius: var(--te-base-border-radius-1); +} :deep(.tiny-tabs__item:first-child) { - border-top-left-radius: var(--te-base-border-radius-1); - border-bottom-left-radius: var(--te-base-border-radius-1); + @extend .tiny-tabs__item--rounded; } :deep(.tiny-tabs__item:last-child) { - border-top-right-radius: var(--te-base-border-radius-1); - border-bottom-right-radius: var(--te-base-border-radius-1); + @extend .tiny-tabs__item--rounded; }
Line range hint
1-116: Consider architectural improvements for maintainability and accessibility.While the component is well-structured, consider these enhancements:
- Add TypeScript for better type safety and developer experience
- Add component documentation using JSDoc or Vue's custom blocks
- Improve accessibility by adding ARIA attributes to tabs
Example documentation format:
<!-- @component Main @description A plugin panel component that displays content in tabs @example <Main :shortcut="false" :fixedPanels="[]" :registryData="{}" @close="handleClose" /> -->Example TypeScript conversion:
interface PluginRegistryData { title?: string options: { displayComponentIds: string[] defaultTabId?: string } components?: { header?: Component } } interface Props { shortcut: boolean | string fixedPanels?: Array<unknown> registryData: PluginRegistryData }packages/plugins/datasource/src/Main.vue (2)
19-22: Consider adding ARIA attributes for better accessibilityWhile the button structure is improved with separate icon and text elements, consider adding ARIA attributes for better screen reader support.
- <tiny-button class="add-data-source" @click="openDataSourceFormPanel()"> + <tiny-button + class="add-data-source" + @click="openDataSourceFormPanel()" + aria-label="添加数据源" + > <svg-icon name="add" class="add-data-source-icon"></svg-icon> <span class="add-data-source-text">添加数据源</span> </tiny-button>
168-173: Improve icon alignment for better visual consistencyThe current
vertical-align: submight cause inconsistent alignment across different browsers. Consider using flexbox for more reliable alignment..add-data-source { height: 24px; color: var(--ti-lowcode-data-source-color); + display: inline-flex; + align-items: center; .add-data-source-icon { font-size: 16px; color: var(--te-common-icon-secondary); margin-right: 4px; - vertical-align: sub; } }packages/common/component/PluginBlockList.vue (1)
497-497: Consider using CSS Grid or Flexbox for responsive layout.The fixed width of 50% for
.item-textmight not be optimal for all screen sizes and content lengths. Consider using a more flexible layout approach.-.item-text { - width: 50%; -} +.item-text { + flex: 1; + min-width: 0; /* Prevent flex item from overflowing */ +}packages/theme/base/src/component-common.less (2)
259-272: Enhanced modal dialog styling for better user experience.Good improvements in the modal dialog:
- Consistent spacing and positioning
- Clear visual hierarchy with bold titles
- Better close button interaction
- Minimum button width ensures consistent appearance
However, consider adding transition animations for the close button hover state.
.tiny-modal__close-btn { top: -4px; right: -4px; + transition: color 0.3s ease; &:hover { color: var(--te-common-icon-primary); } }Also applies to: 392-394, 404-410, 415-415, 422-424
701-705: Consistent form layout improvements.The changes provide better spacing and alignment in forms. However, consider using CSS custom properties for the margin values to maintain consistency.
.tiny-form-item.is-text.is-no-asterisk.tiny-form-item--default { - margin-bottom: 12px; + margin-bottom: var(--te-common-space-3); &:last-of-type { margin-bottom: 0px; } }Also applies to: 714-722
packages/plugins/schema/src/Main.vue (2)
Line range hint
82-91: Enhance schema validation and error handlingThe current schema validation only checks if parsing succeeds. Consider:
- Adding specific validation rules for schema structure
- Providing more detailed error messages that indicate the exact validation failure
- Implementing schema type checking
Here's a suggested improvement:
const saveSchema = () => { const editorValue = string2Obj(app.refs.container.getEditor().getValue()) - if (!editorValue) { + try { + if (!editorValue) { + throw new Error('Empty or invalid schema') + } + + // Add schema structure validation + const requiredFields = ['componentName', 'props', 'children'] + const missingFields = requiredFields.filter(field => !(field in editorValue)) + + if (missingFields.length > 0) { + throw new Error(`Missing required fields: ${missingFields.join(', ')}`) + } + useNotify({ type: 'error', - title: 'schema 保存失败', - message: 'schema 解析异常，请确保格式正确' + title: 'Schema validation failed', + message: `Invalid schema: ${error.message}` }) return + } catch (error) { + console.error('Schema validation error:', error) + return }
144-151: Consider responsive design improvementsThe current fixed positioning and width might cause issues on different screen sizes:
- Fixed
width: 50vwmight be too wide on smaller screens- Fixed
left: 41pxpositioning might not work well with different sidebar widths- Fixed height calculation might not account for varying top panel heights
Consider using:
- CSS Grid or Flexbox for more flexible layouts
- Media queries for responsive adjustments
- CSS Custom Properties for dynamic positioning
#source-code { - width: 50vw; + width: clamp(300px, 50vw, 800px); height: calc(100% - var(--base-top-panel-height)); padding: 12px 0; position: fixed; top: var(--base-top-panel-height); - left: 41px; + left: var(--sidebar-width, 41px); background: var(--ti-lowcode-common-component-bg); box-shadow: 6px 0px 3px 0px rgba(0, 0, 0, 0.05);packages/settings/events/src/components/BindEvents.vue (2)
323-349: Consider consolidating common button stylesThe custom event button and bind event button share similar styles. Consider extracting common styles to reduce duplication and improve maintainability.
+.common-button-styles { + padding: 0 16px; + font-size: 12px; + border-color: var(--te-common-border-default); + &:hover { + border-color: var(--te-common-border-hover); + } +} .add-custom-event-button { + @extend .common-button-styles; - padding: 0 16px; - font-size: 12px; - border-color: var(--te-common-border-default); // ... other specific styles ... - &:hover { - border-color: var(--te-common-border-hover); - } } .bind-event-btn { + @extend .common-button-styles; - padding: 0 16px; - font-size: 12px; - border-color: var(--te-common-border-default); width: 100%; // ... other specific styles ... - &:hover { - border-color: var(--te-common-border-hover); - } }
Line range hint
1-400: Consider component responsibilities and type safetyWhile the current UI changes are good, consider these architectural improvements for future iterations:
The component handles multiple responsibilities (event management, UI, dialogs). Consider splitting it into smaller, focused components:
- EventBindingList (UI rendering)
- EventBindingManager (logic)
- EventDialogManager (dialog handling)
Consider adopting TypeScript to improve type safety, especially for the complex event binding logic and state management.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (16)
packages/common/component/BlockDeployDialog.vue(3 hunks)packages/common/component/PluginBlockList.vue(2 hunks)packages/layout/src/DesignPlugins.vue(1 hunks)packages/layout/src/DesignSettings.vue(1 hunks)packages/plugins/block/src/BlockGroupArrange.vue(1 hunks)packages/plugins/block/src/Main.vue(1 hunks)packages/plugins/block/src/SaveNewBlock.vue(1 hunks)packages/plugins/bridge/src/BridgeSetting.vue(1 hunks)packages/plugins/datasource/src/Main.vue(2 hunks)packages/plugins/materials/src/meta/component/src/Main.vue(1 hunks)packages/plugins/materials/src/meta/layout/src/Main.vue(1 hunks)packages/plugins/schema/src/Main.vue(2 hunks)packages/plugins/state/src/CreateVariable.vue(3 hunks)packages/plugins/state/src/Main.vue(4 hunks)packages/settings/events/src/components/BindEvents.vue(2 hunks)packages/theme/base/src/component-common.less(7 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/block/src/SaveNewBlock.vue
🔇 Additional comments (22)
packages/plugins/state/src/Main.vue (4)
25-28: LGTM! Improved button implementation
The button implementation has been enhanced with better accessibility by:
- Using SVG icon instead of icon component
- Explicitly wrapping text in a span element
- Maintaining dynamic text based on the active state
106-106: LGTM! Component registration is consistent
The component registration aligns with the template usage, maintaining only the necessary icon components.
431-431: LGTM! Consistent spacing
The margin adjustment maintains consistent spacing within the component.
402-414: Verify color contrast ratios for accessibility
While the styling improvements are good, please ensure that the color variables used (--ti-lowcode-chat-model-button-text, --te-common-icon-secondary) provide sufficient contrast ratios for accessibility compliance.
packages/plugins/block/src/BlockGroupArrange.vue (2)
51-57: Improved accessibility and visual consistency
The increased dimensions (24x24px) provide better touch targets, and the use of semantic color variables (--te-common-*) improves theme consistency. The background and border styling create a clear visual hierarchy.
64-66: Enhanced active state styling
The active state styling now properly indicates the selected state through border, color, and background changes, following the design system's color tokens.
packages/layout/src/DesignSettings.vue (1)
108-108: Verify tab title spacing alignment
The increased padding-bottom (6px) improves vertical spacing, but please verify this aligns well with other navigation elements in the design system.
✅ Verification successful
Let me gather more information about the tab-related styling to make a comprehensive verification.
Based on the search results, I can see that the padding-bottom: 6px is only used in the tiny-tabs__item__title class within DesignSettings.vue and there are no other instances of this class with different padding values. The change appears to be isolated and consistent with the component's styling structure.
Padding change is safe to apply
The 6px bottom padding on tab titles is an isolated style that won't affect alignment with other UI elements since:
- It's scoped to the specific component's tab titles
- No other components define conflicting padding for the same class
- The tabs component has a flexible layout system that accommodates varying title sizes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other tab title padding definitions to ensure consistency
rg -l "tiny-tabs__item__title" | xargs rg "padding-bottom"
Length of output: 124
Script:
#!/bin/bash
# Search for any tab-related styling patterns
rg -A 3 "tiny-tabs"
# Also check for similar class patterns
rg -A 3 "__item__title"
Length of output: 69237
packages/plugins/materials/src/meta/component/src/Main.vue (2)
149-149: Improved component spacing consistency
The component item padding now uses the same spacing variable, ensuring consistent vertical rhythm throughout the interface.
139-141: Verify spacing variable definition
The use of --te-common-vertical-form-label-spacing improves consistency, but please verify this variable is properly defined in the theme system.
✅ Verification successful
CSS variable --te-common-vertical-form-label-spacing is properly defined
The spacing variable is defined in the theme system at packages/theme/base/src/common.less with a value of 8px and is already being used consistently in other components for form label spacing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the spacing variable definition
rg --type=less "te-common-vertical-form-label-spacing"
Length of output: 539
packages/common/component/BlockDeployDialog.vue (2)
13-13: LGTM: Label width adjustment
The reduction in label width from 100px to 90px aligns with the PR's UI refinement objectives.
257-261: LGTM: Schema compare button styling
The removal of left padding improves the visual alignment of the compare button.
packages/layout/src/DesignPlugins.vue (1)
204-206: LGTM: Tab navigation background adjustment
Setting the background color to transparent improves visual consistency with the surrounding UI elements.
packages/plugins/block/src/Main.vue (2)
491-491: LGTM: Footer visual separation
The addition of a subtle box shadow effectively separates the footer from the content area.
495-504: LGTM: Dropdown styling refinements
The styling changes improve consistency by:
- Using design system color variables
- Enhancing hover state visibility
- Standardizing icon dimensions
packages/common/component/PluginBlockList.vue (1)
517-519: LGTM: Hover state enhancement improves user interaction feedback.
The addition of hover background color provides clear visual feedback for interactive elements.
packages/theme/base/src/component-common.less (1)
Line range hint 227-252: Improved search component accessibility and visual feedback.
The addition of focus state and consistent spacing improves both usability and visual hierarchy. The placeholder styling enhances readability.
packages/plugins/schema/src/Main.vue (3)
156-158: LGTM: Style improvements align with PR objectives
The border color variable usage and padding adjustments improve consistency with the design system.
198-198: LGTM: Consistent margin
The added margin provides better content spacing.
Line range hint 6-7: Review commented and hidden features
There are several UI elements that appear to be in a transitional state:
- Schema input functionality is temporarily enabled (as noted in comments)
- Import Schema feature is hidden with
v-show="false"
Consider:
- Adding a TODO comment with a tracking issue for re-enabling these features
- Documenting the conditions that need to be met to re-enable these features
Let's check for related TODO items or issues:
Also applies to: 11-17
✅ Verification successful
Commented code reflects intentional feature gating
The commented code and related functionality are intentionally disabled as part of a planned feature rollout strategy. This is evidenced by:
- The schema editing is temporarily enabled as indicated by the
readOnly: falsesetting with a clear comment: "暂时放开schema录入功能，等画布功能完善后，再打开此注释" - The save button's popover is commented out but the save functionality remains accessible
- The import schema feature is intentionally hidden (
v-show="false")
These are not oversights but deliberate feature gates waiting for canvas functionality to be completed. No action is required at this time.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for related TODO comments about schema features
rg -i "todo.*schema"
Length of output: 116
Script:
#!/bin/bash
# Let's search for any references to schema features or canvas functionality
# since the comment mentions waiting for canvas features to be complete
rg -i "canvas.*feature|schema.*feature" -A 2
# Also search for any configuration or feature flags related to schema editing
rg -i "isEdit.*schema|schema.*edit" -A 2
Length of output: 330
Script:
#!/bin/bash
# Let's check the Main.vue file to understand the full context of the schema editing functionality
cat packages/plugins/schema/src/Main.vue
# Also search for any related components that might be using this schema editor
rg -l "Main.vue|SchemaEditor|schema.*editor" --type vue
Length of output: 5863
packages/settings/events/src/components/BindEvents.vue (2)
15-15: LGTM: Icon styling enhancement
The addition of the bind-event-btn-icon class aligns with the PR's UI improvement objectives and maintains consistent styling patterns.
323-337: LGTM: Custom event button styling
The styling implementation properly uses CSS variables for theming and includes appropriate hover states, aligning with the PR's UI improvement objectives.
packages/plugins/bridge/src/BridgeSetting.vue (1)
367-371: Styles Updated for .code-preview Component
The modifications to the .code-preview styles enhance the readability and align with the design specifications.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/theme/base/src/component-common.less (2)
Line range hint
220-252: Consider consolidating search component stylesThe search component styles could be better organized:
- The triple class selector
.tiny-search.tiny-search.tiny-searchis redundant and impacts specificity unnecessarily.- The focus state styles could be combined with the hover state for better maintainability.
Consider this refactor:
-.tiny-search.tiny-search.tiny-search { +.tiny-search { .tiny-search__line { height: 24px; border-radius: var(--te-base-border-radius-1); border: 1px solid var(--te-common-border-default); font-size: var(--te-base-font-size-base); + &:hover, + &.focus { + border-color: var(--te-common-border-active); + } } - .focus { - border-color: var(--te-common-border-active); - }
Line range hint
383-424: Ensure consistent modal box stylingThe modal box styling is comprehensive but has some potential improvements:
- The triple class selector
.tiny-modal__box.tiny-modal__box.tiny-modal__boxis unnecessarily specific- The box-shadow uses a CSS variable but the spread radius is hardcoded
Consider this refactor:
-.tiny-modal__box.tiny-modal__box.tiny-modal__box { +.tiny-modal__box { padding: 20px; border-radius: var(--te-base-border-radius-1); - box-shadow: 0 0 10px 0 var(--te-base-rgba-23); + box-shadow: 0 0 var(--te-base-shadow-spread, 10px) 0 var(--te-base-rgba-23);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/theme/base/src/component-common.less(8 hunks)
🔇 Additional comments (3)
packages/theme/base/src/component-common.less (3)
117-117: LGTM: Margin adjustment for collapse item word overflow
The margin adjustment using CSS variables ensures consistent spacing and aligns with the design system.
701-705: LGTM: Form item margin adjustments
The margin adjustments for form items are well-structured and maintain consistency with the last item having no bottom margin.
259-272: Improve dialog close button accessibility
The close button positioning and interaction states are well defined, but consider adding aria-label support in the component's template for better accessibility.
| .tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label { | ||
| padding-left: 0; | ||
| } | ||
| .tiny-form--label-left.label-align | ||
| .tiny-form-item.is-required | ||
| ~ .tiny-form-item | ||
| .tiny-form-item__label.tiny-form-item__label { | ||
| padding-left: 0; | ||
| } | ||
| .tiny-form-item__label.tiny-form-item__label { | ||
| padding-right: 16px; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
Consider this refactor:
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form--label-left.label-align
- .tiny-form-item.is-required
- ~ .tiny-form-item
- .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form-item__label.tiny-form-item__label {
- padding-right: 16px;
-}
+.tiny-form--label-left.label-align {
+ .tiny-form-item__label {
+ padding-left: 0;
+ padding-right: 16px;
+ }
+}📝 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-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label { | |
| padding-left: 0; | |
| } | |
| .tiny-form--label-left.label-align | |
| .tiny-form-item.is-required | |
| ~ .tiny-form-item | |
| .tiny-form-item__label.tiny-form-item__label { | |
| padding-left: 0; | |
| } | |
| .tiny-form-item__label.tiny-form-item__label { | |
| padding-right: 16px; | |
| } | |
| .tiny-form--label-left.label-align { | |
| .tiny-form-item__label { | |
| padding-left: 0; | |
| padding-right: 16px; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
packages/theme/base/src/component-common.less (4)
117-117: Consider using CSS custom properties for margin valuesThe margin value combines multiple CSS variables. While this works, it might be clearer to create a dedicated custom property for this specific spacing combination.
- margin: var(--te-common-vertical-item-spacing-normal) 0px var(--te-common-vertical-form-label-spacing); + --collapse-item-margin: var(--te-common-vertical-item-spacing-normal) 0 var(--te-common-vertical-form-label-spacing); + margin: var(--collapse-item-margin);
Line range hint
220-253: Improve search component's focus state handlingThe current implementation adds a new
.focusclass for handling focus states. Consider using the native:focus-withinpseudo-class for better accessibility and maintenance.- .focus { - border-color: var(--te-common-border-active); - } + &:focus-within { + border-color: var(--te-common-border-active); + }
Line range hint
384-411: Consolidate modal box shadow valuesThe modal uses a box-shadow with a custom RGBA variable. Consider creating a dedicated shadow token for consistency across modals.
+/* At the top of the file */ +:root { + --te-modal-box-shadow: 0 0 10px 0 var(--te-base-rgba-23); +} .tiny-modal__box.tiny-modal__box.tiny-modal__box { padding: 20px; border-radius: var(--te-base-border-radius-1); - box-shadow: 0 0 10px 0 var(--te-base-rgba-23); + box-shadow: var(--te-modal-box-shadow);
Line range hint
729-802: Improve grid styling maintainabilityThe grid component has deeply nested selectors which could impact maintainability. Consider using BEM methodology more strictly and reducing selector specificity.
.tiny-grid__wrapper.tiny-grid__wrapper.tiny-grid__wrapper { - .tiny-grid { + --grid-cell-height: 30px; + --grid-header-height: 24px; + + &__grid { font-size: var(--te-base-font-size-base); background-color: var(--te-common-bg-default); color: var(--te-common-text-primary); cursor: pointer; + } + + &__header { + background-color: var(--te-common-bg-container); + height: var(--grid-header-height); } /* Continue refactoring other nested selectors similarly */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/theme/base/src/component-common.less(11 hunks)
🔇 Additional comments (2)
packages/theme/base/src/component-common.less (2)
416-425: LGTM: Modal body and footer styles
The changes to modal body padding and footer button sizing look good and maintain consistency with the design system.
Line range hint 702-726: Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
| height: auto; | ||
| font-size: var(--te-base-font-size-1); | ||
| color: var(--te-common-text-primary); | ||
| padding: 0; | ||
| .tiny-dialog-box__close { | ||
| font-size: var(--ti-common-font-size-2); | ||
| color: var(--te-common-text-weaken); | ||
| position: absolute; | ||
| top: -4px; | ||
| right: -4px; | ||
| &:hover { | ||
| color: var(--te-common-icon-primary); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance dialog close button accessibility
The close button positioning and hover state are defined, but it might benefit from additional accessibility improvements.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-text-weaken);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ padding: 4px;
+ border-radius: var(--te-base-border-radius-1);
&:hover {
color: var(--te-common-icon-primary);
+ background-color: var(--te-common-bg-container);
}
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
}📝 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.
| height: auto; | |
| font-size: var(--te-base-font-size-1); | |
| color: var(--te-common-text-primary); | |
| padding: 0; | |
| .tiny-dialog-box__close { | |
| font-size: var(--ti-common-font-size-2); | |
| color: var(--te-common-text-weaken); | |
| position: absolute; | |
| top: -4px; | |
| right: -4px; | |
| &:hover { | |
| color: var(--te-common-icon-primary); | |
| } | |
| } | |
| height: auto; | |
| font-size: var(--te-base-font-size-1); | |
| color: var(--te-common-text-primary); | |
| padding: 0; | |
| .tiny-dialog-box__close { | |
| font-size: var(--ti-common-font-size-2); | |
| color: var(--te-common-text-weaken); | |
| position: absolute; | |
| top: -4px; | |
| right: -4px; | |
| cursor: pointer; | |
| padding: 4px; | |
| border-radius: var(--te-base-border-radius-1); | |
| &:hover { | |
| color: var(--te-common-icon-primary); | |
| background-color: var(--te-common-bg-container); | |
| } | |
| &:focus-visible { | |
| outline: 2px solid var(--te-common-border-active); | |
| outline-offset: 2px; | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
packages/plugins/i18n/src/Main.vue (1)
524-524: Consider accessibility improvement for the download linkWhile the styling changes look good, consider adding
aria-labelto improve accessibility for screen readers.- <a class="download-btn" @click="downloadFile"> 下载导入模板 </a> + <a class="download-btn" @click="downloadFile" aria-label="Download import template"> 下载导入模板 </a>Also applies to: 536-538
packages/theme/base/src/component-common.less (4)
117-117: Consider simplifying margin declarationThe margin declaration combines different semantic spacing variables which could lead to inconsistent spacing across different themes.
- margin: var(--te-common-vertical-item-spacing-normal) 0px var(--te-common-vertical-form-label-spacing); + margin: var(--te-common-vertical-spacing-base) 0;
Line range hint
220-253: LGTM with minor suggestionsThe search component styling improvements look good, with proper focus states and spacing. However, the placeholder font size should use a variable for consistency.
input::-webkit-input-placeholder { color: var(--te-common-text-weaken); - font-size: 12px; + font-size: var(--te-base-font-size-base); }
Line range hint
384-425: Consider using semantic shadow variablesThe modal styling looks good overall, but consider using a semantic shadow variable instead of a direct rgba value.
.tiny-modal__box.tiny-modal__box.tiny-modal__box { padding: 20px; border-radius: var(--te-base-border-radius-1); - box-shadow: 0 0 10px 0 var(--te-base-rgba-23); + box-shadow: var(--te-common-shadow-modal);
Line range hint
729-805: Grid styling improvements neededThe grid styling has a few issues to address:
- Hardcoded border-radius value
- Deep nesting that could be simplified
.tiny-grid-checkbox { color: var(--te-common-border-prompt); - border-radius: 4px; + border-radius: var(--te-base-border-radius-1); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/plugins/i18n/src/Main.vue(7 hunks)packages/theme/base/src/component-common.less(11 hunks)
🔇 Additional comments (6)
packages/plugins/i18n/src/Main.vue (4)
21-23: LGTM: Icon implementation improvement
The switch to svg-icon with consistent styling is a good improvement for maintainability and visual consistency.
472-479: LGTM: Improved search input styling
The adjustments to input prefix alignment and padding enhance the visual presentation of the search box.
490-492: LGTM: Enhanced button styling consistency
Good improvements in button styling:
- Consistent icon sizing and spacing
- Clear disabled state using CSS variables
- Proper border styling for better visual hierarchy
Also applies to: 497-499, 506-506
591-591: LGTM: Consistent icon coloring
Good use of CSS variables for icon colors, maintaining consistency with the theme.
packages/theme/base/src/component-common.less (2)
715-726: 🛠️ Refactor suggestion
Simplify form label padding rules
The current implementation has multiple overlapping selectors for label padding. This could be simplified to reduce specificity and improve maintainability.
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form--label-left.label-align
- .tiny-form-item.is-required
- ~ .tiny-form-item
- .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form-item__label.tiny-form-item__label {
- padding-right: 16px;
-}
+.tiny-form--label-left.label-align {
+ .tiny-form-item__label {
+ padding-left: 0;
+ padding-right: 16px;
+ }
+}260-273: 🛠️ Refactor suggestion
Enhance close button accessibility
While the positioning is good, consider increasing the click target size for better accessibility.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-text-weaken);
position: absolute;
top: -4px;
right: -4px;
+ padding: 8px;
+ cursor: pointer;
&:hover {
color: var(--te-common-icon-primary);
}
}Likely invalid or redundant comment.
| @@ -124,7 +126,7 @@ | |||
| import { computed, ref, watchEffect, reactive, onMounted, nextTick, resolveComponent } from 'vue' | |||
| import useClipboard from 'vue-clipboard3' | |||
| import { Grid, GridColumn, Input, Popover, Button, FileUpload, Loading, Tooltip, Select } from '@opentiny/vue' | |||
| import { iconLoadingShadow, iconPlus, iconUpload } from '@opentiny/vue-icon' | |||
| import { iconLoadingShadow, iconUpload } from '@opentiny/vue-icon' | |||
There was a problem hiding this comment.
💡 Codebase verification
Remove unused icon imports
The iconUpload import appears to be unused. While it's registered in the components section as IconUpload, there's no evidence of its usage in the template. Since TinyFileUpload component is also not used in the template, the iconUpload import can be safely removed.
- Remove
iconUploadfrom the imports on line 129, keeping onlyiconLoadingShadow - Remove the
IconUploadcomponent registration
🔗 Analysis chain
Verify usage of remaining icon imports
While removing iconPlus aligns with the template changes, let's verify the usage of iconUpload.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for iconUpload usage in the component
rg "IconUpload:|iconUpload" packages/plugins/i18n/src/Main.vue
Length of output: 160
Script:
#!/bin/bash
# Search for IconUpload usage in the template section
rg -A 2 -B 2 "IconUpload" packages/plugins/i18n/src/Main.vue
Length of output: 155
Script:
#!/bin/bash
# Search for FileUpload usage in the template to understand if IconUpload is needed
rg -A 5 -B 5 "<TinyFileUpload|<file-upload" packages/plugins/i18n/src/Main.vue
Length of output: 80
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/theme/base/src/component-common.less (3)
117-117: Consider simplifying the margin declarationThe margin declaration could be simplified by using shorthand notation.
-margin: var(--te-common-vertical-item-spacing-normal) 0px var(--te-common-vertical-form-label-spacing); +margin: var(--te-common-vertical-item-spacing-normal) 0 var(--te-common-vertical-form-label-spacing);
Line range hint
220-253: LGTM! Consider enhancing keyboard focus stylesThe changes improve visual feedback, but consider adding a focus outline for better keyboard accessibility.
.focus { border-color: var(--te-common-border-active); + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; }
Line range hint
733-809: Consider using semantic color variablesThe changes improve visual consistency, but some color values could use more semantic variable names.
.tiny-grid-checkbox { - color: var(--te-common-border-prompt); + color: var(--te-grid-checkbox-color); border-radius: 4px; } .row__selected .tiny-grid-checkbox { - color: var(--te-common-color-info); + color: var(--te-grid-checkbox-selected-color); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/plugins/i18n/src/Main.vue(8 hunks)packages/theme/base/src/component-common.less(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/i18n/src/Main.vue
🔇 Additional comments (2)
packages/theme/base/src/component-common.less (2)
264-273: Previous accessibility improvements already suggested
These changes were previously addressed in a past review comment.
Line range hint 693-730: Previous form alignment improvements already suggested
The form label alignment changes were previously addressed in a past review comment. However, the new margin-bottom changes improve spacing consistency.
Consider using CSS Grid for better form layout control:
.tiny-form.tiny-form {
+ display: grid;
+ gap: 12px;
.tiny-form-item {
- margin-bottom: 12px;
- &:last-of-type {
- margin-bottom: 0px;
- }
}
}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/plugins/block/src/Main.vue (1)
528-535: Consider adding focus outline for accessibilityWhile the hover and active states are well-implemented, consider adding a visible outline for the focus state to improve keyboard navigation accessibility.
:deep(.tiny-dropdown-item) { &:not(.is-disabled):active, &:not(.is-disabled):hover, &:not(.is-disabled):focus { background-color: var(--te-common-bg-container); color: var(--te-common-text-primary); + &:focus-visible { + outline: 2px solid var(--te-common-focus-outline-color); + outline-offset: -2px; + } } }packages/plugins/materials/src/meta/block/src/BlockList.vue (1)
Line range hint
177-196: Consider refactoring the deleteBlock function for better maintainability.The
deleteBlockfunction handles multiple responsibilities (confirmation UI, data fetching, and state updates). Consider these improvements:
- Extract the confirmation dialog logic into a separate composable
- Move hardcoded strings to a localization file
- Consider using a dedicated component for the confirmation message instead of inline JSX
Here's a suggested approach:
+ // useBlockConfirmation.js + export function useBlockConfirmation() { + const { confirm } = useModal() + + const confirmBlockDeletion = ({ label, groupName }) => { + return confirm({ + title: `删除区块${label}`, + message: { + render: () => <BlockDeleteConfirmation label={label} groupName={groupName} /> + } + }) + } + + return { confirmBlockDeletion } + } // BlockList.vue - const deleteBlock = ({ id: blockId, label, groupId: id, groupName: name }) => { + const deleteBlock = async ({ id: blockId, label, groupId: id, groupName: name }) => { + const { confirmBlockDeletion } = useBlockConfirmation() const groupId = id || selectedGroup.value.groupId const groupName = name || selectedGroup.value.groupName - const title = `删除区块${label}` - const messageRender = { - render: () => ( - <span> - 您确定将区块 <b>{label}</b> 从分组 <b>{groupName}</b> 中移除吗? - </span> - ) - } - const exec = () => { + try { + await confirmBlockDeletion({ label, groupName }) fetchGroupBlocksById({ groupId })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
packages/common/component/BlockDeployDialog.vue(3 hunks)packages/plugins/block/src/CategoryEdit.vue(1 hunks)packages/plugins/block/src/Main.vue(3 hunks)packages/plugins/block/src/SaveNewBlock.vue(2 hunks)packages/plugins/materials/src/meta/block/src/BlockList.vue(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/block/src/CategoryEdit.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/common/component/BlockDeployDialog.vue
- packages/plugins/block/src/SaveNewBlock.vue
🔇 Additional comments (1)
packages/plugins/block/src/Main.vue (1)
491-504: LGTM: Improved visual hierarchy and theme consistency
The styling changes effectively enhance the visual separation of the block footer while maintaining consistency with the design system through proper use of CSS variables.
| @@ -193,7 +192,7 @@ export default { | |||
| message({ message: `删除区块失败: ${error.message || error}`, status: 'error' }) | |||
| }) | |||
| } | |||
| confirm({ title, status, message: messageRender, exec }) | |||
| confirm({ title, message: messageRender, exec }) | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent removal of status parameter in confirmation dialogs
The codebase shows that the status parameter is consistently used in other confirmation dialogs:
packages/common/component/Modal.jsxdefinesconfirmwithstatusas a parameter- Multiple files still use
statusin their confirm calls:packages/plugins/page/src/PageSetting.vuepackages/plugins/materials/src/meta/block/src/BlockGroup.vuepackages/plugins/block/src/BlockSetting.vuepackages/plugins/materials/src/meta/block/src/BlockDetail.vue
The removal of status in BlockList.vue creates an inconsistency with the rest of the codebase and the function's signature.
🔗 Analysis chain
LGTM! Simplified confirmation dialog call.
The removal of the status parameter from the confirm call is appropriate as it's not needed for confirmation dialogs. The error handling is properly maintained in the promise chain.
Let's verify that the confirmation dialog still appears correctly:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other usages of confirm with status parameter
# to ensure this change is consistent across the codebase
# Search for confirm calls with status parameter
rg "confirm.*status.*" --type vue
# Search for similar confirmation dialogs in block management
rg "confirm.*message.*exec.*" --type vue
Length of output: 134
Script:
#!/bin/bash
# Let's try searching without file type restrictions since 'vue' type is not recognized
# Search for confirm calls with status parameter
rg "confirm.*status.*"
# Search for similar confirmation dialogs in block management
rg "confirm.*message.*exec.*"
# Also search for the confirm function definition to understand its signature
ast-grep --pattern 'confirm = function($_) { $$$ }'
ast-grep --pattern 'function confirm($_) { $$$ }'
ast-grep --pattern 'confirm: function($_) { $$$ }'
Length of output: 1712
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/plugins/block/src/Main.vue (1)
536-538: Consider using a shared CSS custom property for dropdown menu paddingTo avoid repetition and improve maintainability, consider defining a shared CSS custom property for dropdown menu padding.
+:root { + --te-dropdown-menu-padding: var(--te-common-vertical-form-label-spacing) 0; +} -:deep(.tiny-dropdown-menu.tiny-popper[x-placement^='top']) { - padding: var(--te-common-vertical-form-label-spacing) 0; -} +:deep(.tiny-dropdown-menu.tiny-popper[x-placement^='top']) { + padding: var(--te-dropdown-menu-padding); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/plugins/block/src/Main.vue(3 hunks)
🔇 Additional comments (4)
packages/plugins/block/src/Main.vue (4)
491-504: LGTM: Footer styling improvements enhance visual hierarchy
The added box shadow and color styling improvements help distinguish the footer section while maintaining consistency with the design system through CSS custom properties.
505-507: Remove duplicate dropdown menu padding styles
The padding style for .tiny-dropdown-menu is defined multiple times in the file. Consider removing the duplicate styles to improve maintainability.
528-535: LGTM: Well-defined dropdown item states
The hover, active, and focus states are properly implemented using design system colors, which improves accessibility and user interaction feedback.
579-581: Remove duplicate global dropdown menu styles
This is another instance of duplicate padding styles. Consider consolidating these styles with the other dropdown menu padding definitions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
packages/plugins/materials/src/meta/layout/src/Main.vue (2)
111-119: LGTM! Consider consolidating border radius declarations.The border radius styling for tabs looks good and aligns with the UI enhancement objectives. However, the code could be more DRY.
Consider consolidating the border radius declarations:
-:deep(.tiny-tabs__item:first-child) { - border-top-left-radius: var(--te-base-border-radius-1); - border-bottom-left-radius: var(--te-base-border-radius-1); -} - -:deep(.tiny-tabs__item:last-child) { - border-top-right-radius: var(--te-base-border-radius-1); - border-bottom-right-radius: var(--te-base-border-radius-1); -} +:deep(.tiny-tabs__item) { + &:first-child { + border-radius: var(--te-base-border-radius-1) 0 0 var(--te-base-border-radius-1); + } + + &:last-child { + border-radius: 0 var(--te-base-border-radius-1) var(--te-base-border-radius-1) 0; + } +}
111-119: Consider enhancing tab accessibility.While the visual styling is good, consider adding hover and focus states for better accessibility.
Add hover and focus states:
+:deep(.tiny-tabs__item) { + &:hover { + background-color: var(--te-base-hover-background); + } + + &:focus-visible { + outline: 2px solid var(--te-base-focus-ring); + outline-offset: -2px; + } +}packages/common/component/PluginBlockList.vue (3)
56-65: Add accessibility attributes to SVG buttonsThe SVG buttons should include ARIA labels and roles for better accessibility. Also, consider using more descriptive event handler names.
- <li class="list-item" @mousedown.stop.left="editBlock({ event: $event, item, index })"> - <svg-button class="list-item-svg" name="to-edit"> </svg-button> + <li class="list-item" @mousedown.stop.left="handleEditBlock({ event: $event, item, index })"> + <svg-button + class="list-item-svg" + name="to-edit" + role="button" + aria-label="Edit block" + > </svg-button> </li> <li class="list-item" @mouseover.stop="iconSettingMove" - @mousedown.stop.prevent="iconClick({ event: $event, item, index })" + @mousedown.stop.prevent="handleSettingsClick({ event: $event, item, index })" > - <svg-button class="list-item-svg" name="text-source-setting"> </svg-button> + <svg-button + class="list-item-svg" + name="text-source-setting" + role="button" + aria-label="Block settings" + > </svg-button>
Line range hint
493-515: Enhance hover interaction and improve responsive designThe hover effect could benefit from a smooth transition, and the fixed width percentage might cause issues with responsive layouts.
.item-text { - width: 50%; + width: clamp(200px, 50%, 400px); } .block-detail, .block-setting { visibility: hidden; position: static; margin-left: 4px; z-index: 9; .block-detail-icon { color: var(--ti-lowcode-component-block-list-setting-btn-color); display: block; &:hover { cursor: pointer; color: var(--ti-lowcode-component-block-list-setting-btn-hover-color); } } } &:hover { background-color: var(--te-common-bg-container); + transition: background-color 0.2s ease; }
Line range hint
1-24: Consider performance optimizations for hover effects and event handlersThe component uses multiple hover effects and event handlers which could impact performance. Consider:
- Debouncing the hover event handlers
- Using CSS
pointer-events: nonefor inactive elements- Implementing virtual scrolling for large lists
Would you like me to provide an example implementation of these optimizations?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
packages/common/component/PluginBlockList.vue(3 hunks)packages/layout/src/DesignPlugins.vue(1 hunks)packages/plugins/materials/src/meta/layout/src/Main.vue(1 hunks)packages/theme/base/src/component-common.less(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/layout/src/DesignPlugins.vue
- packages/theme/base/src/component-common.less
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/toolbars/clean/src/Main.vue (1)
Line range hint
42-57: Consider enhancing the confirmation UX.While the modal implementation is functional, consider these UX improvements:
- Add a warning icon to visually indicate the destructive nature of the action
- Include a brief explanation of the consequences
Here's a suggested enhancement:
confirm({ title: '提示', message: () => { return [ <div class="modal-content"> { <div class="wrap"> + <i class="tiny-icon-warning warning-icon"></i> - <span>{`您确定要清除屏幕吗？`}</span> + <span> + {`您确定要清除屏幕吗？`} + <div class="sub-text">此操作将清除画布上的所有内容且不可撤销。</div> + </span> </div> } </div> ] } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/toolbars/clean/src/Main.vue(3 hunks)
🔇 Additional comments (3)
packages/toolbars/clean/src/Main.vue (3)
9-10: LGTM! Good use of composition API.
The switch to composition API hooks (useModal) from direct component imports is a good modernization that aligns with Vue best practices.
28-28: LGTM! Clean modal setup.
The destructuring of confirm from useModal provides a cleaner and more maintainable approach.
Line range hint 41-58: Verify error handling for canvas clearing operation.
The canvas clearing operation seems to lack error handling. We should verify if clearCanvas() has proper error handling internally.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
packages/common/component/PluginBlockList.vue (2)
57-66: Add accessibility attributes to menu itemsThe menu structure is clean and well-organized, but could benefit from improved accessibility.
Add aria-labels to the buttons:
- <svg-button class="list-item-svg" name="to-edit"> </svg-button> + <svg-button class="list-item-svg" name="to-edit" aria-label="Edit block"> </svg-button> - <svg-button class="list-item-svg" name="text-source-setting"> </svg-button> + <svg-button class="list-item-svg" name="text-source-setting" aria-label="Block settings"> </svg-button>
515-517: Add fallback color for hover stateThe hover state uses a CSS variable without a fallback color, which could cause issues if the variable is not defined.
Add a fallback color:
&:hover { - background-color: var(--te-common-bg-container); + background-color: var(--te-common-bg-container, #f5f7fa); }packages/theme/base/src/component-common.less (4)
405-411: Improve modal close button accessibilitySimilar to the dialog close button, the modal close button can be enhanced for better accessibility and usability.
Consider adding
cursor: pointer,padding, andborder-radius, and providing a focus indicator for keyboard navigation.Apply this diff to enhance the close button:
.tiny-modal__close-btn { top: -4px; right: -4px; + cursor: pointer; + padding: 4px; + border-radius: var(--te-base-border-radius-1); &:hover { color: var(--te-common-icon-primary); + background-color: var(--te-common-bg-container); } + &:focus-visible { + outline: 2px solid var(--te-common-border-active); + outline-offset: 2px; + } }
250-253: Ensure cross-browser support for placeholder stylingThe use of
::-webkit-input-placeholderonly styles the placeholder text in WebKit browsers. To ensure consistent placeholder styling across all browsers, consider adding the standard::placeholderpseudo-element and other vendor prefixes.Apply the following changes:
input::-webkit-input-placeholder { color: var(--te-common-text-weaken); font-size: 12px; } input::-moz-placeholder { color: var(--te-common-text-weaken); font-size: 12px; } input:-ms-input-placeholder { color: var(--te-common-text-weaken); font-size: 12px; } input::placeholder { color: var(--te-common-text-weaken); font-size: 12px; }
Line range hint
697-714: Consolidate margin-bottom styles for form itemsThere are repeated
margin-bottomstyles for.tiny-form-itemand.tiny-form-item.is-text.is-no-asterisk.tiny-form-item--default. Consolidating these rules can reduce redundancy and improve maintainability.Consider merging the styles as follows:
.tiny-form-item, .tiny-form-item.is-text.is-no-asterisk.tiny-form-item--default { margin-bottom: 12px; &:last-of-type { margin-bottom: 0px; } }
874-890: Maintain consistency with BEM naming conventionsIn the
.tiny-textareacomponent, ensure that class names adhere to the BEM (Block Element Modifier) naming convention for consistency across the codebase.For example, verify that
.tiny-textarea__inneris correctly used as an element of.tiny-textareaand that this pattern is consistently applied throughout the styles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
packages/common/component/BlockDeployDialog.vue(4 hunks)packages/common/component/PluginBlockList.vue(4 hunks)packages/plugins/datasource/src/DataSourceRemoteAutoload.vue(1 hunks)packages/theme/base/src/component-common.less(13 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/datasource/src/DataSourceRemoteAutoload.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/common/component/BlockDeployDialog.vue
🔇 Additional comments (3)
packages/common/component/PluginBlockList.vue (1)
472-482: LGTM: Improved styling for better responsiveness
The styling changes improve the UI with:
- Better positioned publish flag with proper spacing and rounded corners
- More responsive layout using percentage-based width
Also applies to: 495-495
packages/theme/base/src/component-common.less (2)
260-273: Enhance dialog close button accessibility
The close button positioning and hover state are defined, but it might benefit from additional accessibility improvements.
Consider adding cursor: pointer, padding, and border-radius to improve usability. Additionally, include a focus indicator for keyboard navigation.
Apply this diff to enhance the close button:
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-text-weaken);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ padding: 4px;
+ border-radius: var(--te-base-border-radius-1);
&:hover {
color: var(--te-common-icon-primary);
+ background-color: var(--te-common-bg-container);
}
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
}723-734: Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
Consider refactoring the code as follows:
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form--label-left.label-align
- .tiny-form-item.is-required
- ~ .tiny-form-item
- .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form-item__label.tiny-form-item__label {
- padding-right: 16px;
-}
+.tiny-form--label-left.label-align {
+ .tiny-form-item__label {
+ padding-left: 0;
+ padding-right: 16px;
+ }
+}| <!-- <div v-if="isBlockManage && !item.is_published" class="publish-flag">未发布</div> --> | ||
| <div class="publish-flag">未发布</div> |
There was a problem hiding this comment.
Critical: Incorrect publication status display
The change makes the "未发布" (unpublished) flag always visible, regardless of the actual publication status. This could mislead users about whether a block is published or not.
Restore the conditional rendering to show the correct status:
- <!-- <div v-if="isBlockManage && !item.is_published" class="publish-flag">未发布</div> -->
- <div class="publish-flag">未发布</div>
+ <div v-if="!item.is_published" class="publish-flag">未发布</div>📝 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.
| <!-- <div v-if="isBlockManage && !item.is_published" class="publish-flag">未发布</div> --> | |
| <div class="publish-flag">未发布</div> | |
| <div v-if="!item.is_published" class="publish-flag">未发布</div> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/common/component/PluginBlockList.vue (2)
471-481: Improve publish flag styling implementationThe current styling approach has potential issues with text scaling and positioning.
Consider these improvements:
.publish-flag { position: absolute; left: 4px; top: 4px; text-align: center; display: block; color: var(--te-common-text-primary); font-size: 12px; - background-color: var(--te-common-bg-prompt); + background-color: var(--te-status-warning-bg); padding: 2px; border-radius: 4px; - transform: scale(0.9); + font-size: 11px; min-width: 40px; }
494-494: Enhance responsive design and hover interactionsThe current implementation could be improved for better responsiveness and user experience.
Consider these improvements:
.item-text { - width: 50%; + width: clamp(200px, 50%, 400px); } &:hover { background-color: var(--te-common-bg-container); + transition: background-color 0.2s ease; }Also applies to: 514-516
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/common/component/PluginBlockList.vue(4 hunks)
🔇 Additional comments (1)
packages/common/component/PluginBlockList.vue (1)
471-481:
Critical: Incorrect publication status display
The publication flag is still being displayed unconditionally, which could mislead users about the actual publication status of blocks.
| <li class="list-item" @mousedown.stop.left="editBlock({ event: $event, item, index })"> | ||
| <svg-button class="list-item-svg" name="to-edit"> </svg-button> | ||
| </li> | ||
| <li | ||
| class="list-item" | ||
| @mouseover.stop="iconSettingMove" | ||
| @mousedown.stop.prevent="iconClick({ event: $event, item, index })" | ||
| > | ||
| <svg-button class="list-item-svg" name="text-source-setting"> </svg-button> | ||
| </li> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to SVG buttons
The SVG buttons should be enhanced with proper accessibility attributes to ensure they are usable by screen readers and keyboard navigation.
Apply these improvements:
- <li class="list-item" @mousedown.stop.left="editBlock({ event: $event, item, index })">
- <svg-button class="list-item-svg" name="to-edit"> </svg-button>
+ <li class="list-item" @click.stop="editBlock({ event: $event, item, index })"
+ role="button"
+ tabindex="0"
+ @keydown.enter="editBlock({ event: $event, item, index })">
+ <svg-button class="list-item-svg" name="to-edit" aria-label="Edit block"> </svg-button>
</li>
<li
class="list-item"
+ role="button"
+ tabindex="0"
- @mouseover.stop="iconSettingMove"
- @mousedown.stop.prevent="iconClick({ event: $event, item, index })"
+ @click.stop="iconClick({ event: $event, item, index })"
+ @keydown.enter="iconClick({ event: $event, item, index })"
>
- <svg-button class="list-item-svg" name="text-source-setting"> </svg-button>
+ <svg-button class="list-item-svg" name="text-source-setting" aria-label="Block settings"> </svg-button>📝 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.
| <li class="list-item" @mousedown.stop.left="editBlock({ event: $event, item, index })"> | |
| <svg-button class="list-item-svg" name="to-edit"> </svg-button> | |
| </li> | |
| <li | |
| class="list-item" | |
| @mouseover.stop="iconSettingMove" | |
| @mousedown.stop.prevent="iconClick({ event: $event, item, index })" | |
| > | |
| <svg-button class="list-item-svg" name="text-source-setting"> </svg-button> | |
| </li> | |
| <li class="list-item" @click.stop="editBlock({ event: $event, item, index })" | |
| role="button" | |
| tabindex="0" | |
| @keydown.enter="editBlock({ event: $event, item, index })"> | |
| <svg-button class="list-item-svg" name="to-edit" aria-label="Edit block"> </svg-button> | |
| </li> | |
| <li | |
| class="list-item" | |
| role="button" | |
| tabindex="0" | |
| @click.stop="iconClick({ event: $event, item, index })" | |
| @keydown.enter="iconClick({ event: $event, item, index })" | |
| > | |
| <svg-button class="list-item-svg" name="text-source-setting" aria-label="Block settings"> </svg-button> | |
| </li> |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (12)
packages/plugins/help/src/HelpIcon.vue (3)
12-12: Consider internationalizing the title attributeThe hardcoded Chinese text "帮助" in the title attribute should be internationalized for better maintainability and multi-language support.
- <div title="帮助" class="help-plugin-reference"> + <div :title="$t('help')" class="help-plugin-reference">
267-269: Consider adding flex alignment propertiesThe flex container could benefit from explicit alignment properties to ensure consistent icon positioning.
.help-plugin-reference { display: flex; + align-items: center; + justify-content: center; }
Line range hint
1-391: Consider architectural improvements for better maintainabilitySeveral aspects of the component could be enhanced:
- Move URLs and question list to a configuration file:
// config/help.js export const HELP_CONFIG = { courseUrl: 'https://opentiny.design/tiny-engine#/help-center/course/engine', questionList: [ { label: 'help.thirdParty.title', url: 'https://opentiny.design/tiny-engine#/help-center/course/engine/15' }, // ... ] }
- The version comparison logic could be improved to handle semantic versioning properly:
import semver from 'semver' // ... if (!localStorageVersion || semver.lt(localStorageVersion, GUIDE_VERSION)) { toShowStep() }
- Consider extracting the guide tour configuration to a separate file for better maintainability.
Would you like me to help create these configuration files or implement any of these suggestions?
packages/theme/base/src/component-common.less (3)
Line range hint
220-253: Enhance search component accessibilityWhile the styling improvements are good, consider adding focus styles for better keyboard navigation.
.tiny-search.tiny-search.tiny-search { .tiny-search__line { height: 24px; border-radius: var(--te-base-border-radius-1); border: 1px solid var(--te-common-border-default); font-size: var(--te-base-font-size-base); + &:focus-within { + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; + } }
Line range hint
305-361: Enhance button component stylingThe button styling improvements are good, but consider adding transition effects for smoother hover states.
.tiny-button.tiny-button { + transition: all 0.3s ease; display: inline-flex; align-items: center; justify-content: center;
Line range hint
754-840: Enhance grid component interaction statesThe hover and selected states are well-defined, but consider adding focus states for better keyboard navigation.
.tiny-grid-body__column, .tiny-grid-footer__column { height: 30px; text-align: left; border-bottom-color: var(--te-common-border-divider); &:hover { background-color: var(--te-common-bg-container); } + &:focus-within { + outline: 2px solid var(--te-common-border-active); + outline-offset: -2px; + }packages/common/component/LifeCycles.vue (2)
75-75: Consider migrating all icons to svg-icon for consistencyThe component currently mixes two icon systems:
- New:
<svg-icon>for the add button- Old:
iconYesfor selection indicationConsider migrating all icons to use the new
svg-iconcomponent for better maintainability and consistency.-import { iconYes } from '@opentiny/vue-icon' export default { components: { // ... - IconYes: iconYes() } }Then update the template:
-<icon-yes v-if="item === state.title" class="life-cycle-selected__icon"></icon-yes> +<svg-icon v-if="item === state.title" name="yes" class="life-cycle-selected__icon"></svg-icon>Also applies to: 90-90
Line range hint
166-174: Consider enhancing error feedback UXThe error handling for code validation could be improved. Currently, errors are only shown when attempting to save:
if (state.hasError) { useNotify({ type: 'error', message: '代码静态检查有错误，请先修改后再保存' }) return }Consider:
- Showing validation errors in real-time as the user types
- Disabling the save button when there are errors
- Providing more specific error messages
packages/plugins/datasource/src/DataSourceRecordList.vue (4)
15-15: Consider enhancing button accessibilityWhile replacing icon components with svg-icon is good for consistency, consider adding aria-label to the button for better accessibility.
-<tiny-button plain :disabled="!allowCreate" @click.stop="insertNewData" +<tiny-button plain :disabled="!allowCreate" @click.stop="insertNewData" aria-label="Add new static data" ><svg-icon name="add" class="btn-icon"></svg-icon>新增静态数据</tiny-button
Line range hint
108-114: Remove unused component registrationAfter standardizing the icon usage, the IconUpload component registration can be removed.
export default { components: { TinyGrid: Grid, PluginSetting, TinyPager: Pager, DataSourceRecordUpload, TinyLink: Link, TinyButton: Button, - IconUpload: iconUpload() },
Line range hint
365-384: Enhance error handling for data operationsThe saveRecordFormData method could benefit from more robust error handling to provide better feedback to users when operations fail.
const saveRecordFormData = (data) => { const { name, id, data: { columns, type, ...other } } = props.data const requestData = { name, id, data: { columns, type, ...other, data } } - requestUpdateDataSource(props.data.id, requestData).then((res) => { - if (!res) { - return - } + requestUpdateDataSource(props.data.id, requestData).then((res) => { + if (!res) { + useNotify({ + type: 'error', + message: '数据源修改失败' + }) + return + } const key = `datasource${capitalize(camelize(name))}` const pageSchema = useCanvas().canvasApi.value.getSchema() if (pageSchema.state[key]) { pageSchema.state[key] = data.map(({ _id, ...other }) => other) } useNotify({ type: 'success', message: '数据源修改成功' }) fetchData(true) + }).catch(error => { + useNotify({ + type: 'error', + message: `数据源修改失败: ${error.message}` + }) }) }
Line range hint
291-304: Optimize data fetching and watch handlersConsider implementing debouncing for data fetching operations and optimizing the watch handler to prevent unnecessary updates.
- Import debounce utility:
import { debounce } from 'lodash-es'
- Optimize the watch handler:
- watch( - () => props.data.name, - () => { + watch( + () => props.data.name, + (newName, oldName) => { + if (newName === oldName) return state.totalData = [] state.pagerConfig.currentPage = 1 fetchData(true).then(() => { grid.value?.clearAll?.() grid.value?.resetAll?.() grid.value?.validate?.() }) } )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
packages/common/component/LifeCycles.vue(3 hunks)packages/common/component/PluginBlockList.vue(6 hunks)packages/layout/src/DesignPlugins.vue(2 hunks)packages/plugins/datasource/src/DataSourceField.vue(1 hunks)packages/plugins/datasource/src/DataSourceRecordList.vue(2 hunks)packages/plugins/datasource/src/Main.vue(1 hunks)packages/plugins/help/src/HelpIcon.vue(2 hunks)packages/plugins/i18n/src/Main.vue(9 hunks)packages/settings/events/src/components/BindEvents.vue(2 hunks)packages/theme/base/src/component-common.less(15 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/plugins/datasource/src/DataSourceField.vue
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/settings/events/src/components/BindEvents.vue
- packages/layout/src/DesignPlugins.vue
- packages/plugins/datasource/src/Main.vue
- packages/common/component/PluginBlockList.vue
- packages/plugins/i18n/src/Main.vue
🔇 Additional comments (6)
packages/theme/base/src/component-common.less (5)
117-117: LGTM: Consistent spacing in collapse component
The margin adjustment for word overflow items follows the design system's spacing variables.
Line range hint 401-446: LGTM: Modal styling improvements
The modal styling changes improve visual hierarchy and spacing. The close button positioning and styling are consistent with the dialog component.
Line range hint 714-751: Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
889-907: LGTM: Textarea component styling
The textarea component styling is consistent with other form controls and follows the design system's variables.
260-273: 🛠️ Refactor suggestion
Improve dialog close button accessibility
The close button needs better accessibility support for keyboard users and screen readers.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-text-weaken);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ padding: 8px;
+ border-radius: var(--te-base-border-radius-1);
+ aria-label: "Close dialog"
&:hover {
color: var(--te-common-icon-primary);
+ background-color: var(--te-common-bg-container);
}
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
}Likely invalid or redundant comment.
packages/common/component/LifeCycles.vue (1)
5-5: Verify icon accessibility and visual consistency
The change from <icon-plus> to <svg-icon> should maintain proper accessibility. Ensure that:
- The SVG icon has proper ARIA attributes
- The icon size and color match the design system
✅ Verification successful
Based on the extensive search results, I can now provide a final response:
Icon change is part of a consistent design system update
The change from <icon-plus> to <svg-icon> is part of a systematic update across the codebase. The search results show that:
- The
<svg-icon>component is widely used throughout the application with consistent implementation - The component accepts standard properties like
name,width,height, andfillfor styling - The "add" icon is used consistently in similar contexts (e.g., "添加" actions) across multiple components
The change maintains visual consistency and follows the established pattern in the codebase. No accessibility issues are apparent as the icon is used within a button that includes clear text labels ("添加页面生命周期").
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the svg-icon component includes accessibility attributes
ast-grep --pattern 'const $name = defineComponent({
$$$
props: {
$$$
"aria-label": {
$$$
}
$$$
}
$$$
})'
Length of output: 136
Script:
#!/bin/bash
# Let's try a different approach to check the svg-icon component implementation
# First, find the svg-icon component file
fd "svg-icon" --type f
# Then check its content and props
rg -A 10 "defineComponent|props:" --type vue --type ts --type js
# Also check if there are any aria-related attributes in the component usage
rg -B 2 -A 2 "<svg-icon" --type vue
Length of output: 237
Script:
#!/bin/bash
# Let's try without file type restrictions since 'vue' type isn't recognized
rg -B 2 -A 2 "<svg-icon"
# Also check the svg-icon.json file we found
cat "packages/settings/design/src/properties/svg-icon.json"
# Search for any SVG-related component definitions
rg -A 10 "defineComponent.*SvgIcon|props.*SvgIcon"
Length of output: 57180
| color: var(--ti-lowcode-common-primary-color); | ||
| border: 1px solid var(--te-common-border-active); | ||
| color: var(--te-common-text-primary); | ||
| background-color: var(--te-common-bg-default); |
packages/plugins/block/src/Main.vue
Outdated
| @@ -488,9 +488,23 @@ export default { | |||
| right: 0; | |||
| padding: 10px 16px; | |||
| background-color: var(--ti-lowcode-component-search-bg); | |||
| box-shadow: 0 -2px 12px 0 rgba(0, 0, 0, 0.16); | |||
There was a problem hiding this comment.
box-shadow可以用base里面的box-shadow变量
packages/plugins/schema/src/Main.vue
Outdated
| z-index: 1000; | ||
| .source-code-header { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| border-bottom: 1px solid var(--ti-lowcode-toolbar-border-color); | ||
| align-items: center; | ||
| border-bottom: 1px solid var(--ti-lowcode-base-split-color-1); |
There was a problem hiding this comment.
这里看一下是不是可以用 --te-common-border-divider分割线
| } | ||
| .tiny-grid-resizable.is__line:before, | ||
| .tiny-grid-thead-partition.is__line:before { | ||
| background-color: var(--te-common-border-hover); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
packages/plugins/datasource/src/DataSourceRecordList.vue (2)
Line range hint
15-23: Standardize icon component usage across the templateThere's inconsistent usage of icon components in the buttons:
- Some use
<svg-icon>(add, delete)- Others use the old format (icon-upload, icon-download)
Apply these changes for consistency:
-<icon-upload class="btn-icon"></icon-upload>批量导入</tiny-button> +<svg-icon name="upload" class="btn-icon"></svg-icon>批量导入</tiny-button> -<icon-download class="tiny-svg-size icon-download"></icon-download>下载导入模板</tiny-link> +<svg-icon name="download" class="tiny-svg-size icon-download"></svg-icon>下载导入模板</tiny-link>
Line range hint
419-425: Fix potential data sync issue in editClosedThe current implementation syncs data before validation, which could lead to invalid data being stored in
totalData.Consider moving the sync after validation:
const editClosed = () => { grid.value.validate((valid) => { - syncDataToTotalData() if (valid) { + syncDataToTotalData() fetchData() } }) }packages/plugins/block/src/Main.vue (1)
Line range hint
267-279: Consider improving close button accessibilityWhile the positioning and hover state are good, consider enhancing the close button's accessibility:
.tiny-dialog-box__close { font-size: var(--ti-common-font-size-2); color: var(--te-common-icon-secondary); position: absolute; top: -4px; right: -4px; + padding: 4px; + border-radius: var(--te-base-border-radius-1); &:hover { color: var(--te-common-icon-primary); + background-color: var(--te-common-bg-container); } + &:focus-visible { + outline: 2px solid var(--te-common-border-active); + outline-offset: 2px; + } }packages/theme/base/src/component-common.less (1)
782-789: Use semantic background color variableThe background color should use a semantic background variable instead of a border color variable.
.tiny-grid-resizable.is__line:before, .tiny-grid-thead-partition.is__line:before { - background-color: var(--te-common-border-hover); + background-color: var(--te-common-bg-container); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/design-core/assets/plugin-icon-js.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
packages/common/component/BlockDeployDialog.vue(4 hunks)packages/plugins/block/src/BlockSetting.vue(0 hunks)packages/plugins/block/src/Main.vue(3 hunks)packages/plugins/datasource/src/DataSourceRecordList.vue(2 hunks)packages/settings/events/src/components/BindEvents.vue(2 hunks)packages/theme/base/src/component-common.less(15 hunks)
💤 Files with no reviewable changes (1)
- packages/plugins/block/src/BlockSetting.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/settings/events/src/components/BindEvents.vue
- packages/common/component/BlockDeployDialog.vue
🔇 Additional comments (7)
packages/plugins/block/src/Main.vue (3)
Line range hint 120-120: LGTM: Improved spacing consistency
Good use of design system variables for consistent spacing in the collapse header.
Line range hint 226-259: LGTM: Enhanced search component UX
Good improvements to the search component:
- Added focus state for better visual feedback
- Improved prefix icon alignment and color
- Consistent placeholder text styling
537-539: Remove duplicate dropdown menu padding styles
This issue was previously identified. Please refer to the existing comment about removing duplicate padding styles for .tiny-dropdown-menu.
packages/theme/base/src/component-common.less (4)
726-729: LGTM: Improved form spacing consistency
Good improvement to form spacing with proper handling of the last form item.
752-763: Simplify form label alignment rules
This issue was previously identified. Please refer to the existing comment about simplifying the form label alignment rules to reduce specificity and improve maintainability.
837-844: LGTM: Enhanced grid interactivity
Good improvements to the grid component:
- Added hover state for better visual feedback
- Consistent styling for grid inputs
904-921: LGTM: Well-structured textarea implementation
Excellent implementation of the textarea component:
- Follows design system patterns
- Includes all necessary states (hover, focus, disabled)
- Uses semantic color variables consistently
| @@ -81,7 +81,7 @@ | |||
| import { reactive, ref, watchEffect, watch, computed } from 'vue' | |||
| import { camelize, capitalize } from '@vue/shared' | |||
| import { Grid, Pager, Input, Numeric, DatePicker, Switch, Slider, Link, Button } from '@opentiny/vue' | |||
| import { iconPlus, iconUpload } from '@opentiny/vue-icon' | |||
| import { iconUpload } from '@opentiny/vue-icon' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused icon imports after standardization
Once the icon components are standardized to use svg-icon, the following can be removed:
-import { iconUpload } from '@opentiny/vue-icon'
export default {
components: {
TinyGrid: Grid,
PluginSetting,
TinyPager: Pager,
DataSourceRecordUpload,
TinyLink: Link,
TinyButton: Button,
- IconUpload: iconUpload()
},Also applies to: 107-107
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/theme/base/src/component-common.less (2)
Line range hint
226-259: Consider enhancing keyboard focus visibilityThe changes look good, especially the addition of focus states. However, consider adding a visible focus outline for better keyboard accessibility.
.focus { border-color: var(--te-common-border-active); + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; }
837-844: Add transitions for smoother hover effectsConsider adding transitions for background-color changes to make the hover effect smoother.
&:hover { background-color: var(--te-common-bg-container); + transition: background-color 0.2s ease; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
packages/common/component/BlockDeployDialog.vue(4 hunks)packages/plugins/block/src/BlockGroupArrange.vue(1 hunks)packages/plugins/block/src/Main.vue(3 hunks)packages/plugins/schema/src/Main.vue(2 hunks)packages/plugins/script/src/Main.vue(3 hunks)packages/theme/base/src/component-common.less(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/plugins/schema/src/Main.vue
- packages/plugins/script/src/Main.vue
- packages/common/component/BlockDeployDialog.vue
- packages/plugins/block/src/BlockGroupArrange.vue
- packages/plugins/block/src/Main.vue
🔇 Additional comments (6)
packages/theme/base/src/component-common.less (6)
Line range hint 312-368: LGTM! Good improvements to button layout
The addition of flex display and consistent icon spacing improves button alignment and visual consistency.
Line range hint 408-444: LGTM! Good typography improvements
The changes establish a clear visual hierarchy with appropriate font weights and spacing.
904-921: LGTM! Consistent with input styles
The textarea styles maintain consistency with input component styles, which is good for UX.
752-763: 🛠️ Refactor suggestion
Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form--label-left.label-align
- .tiny-form-item.is-required
- ~ .tiny-form-item
- .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form-item__label.tiny-form-item__label {
- padding-right: 16px;
-}
+.tiny-form--label-left.label-align {
+ .tiny-form-item__label {
+ padding-left: 0;
+ padding-right: 16px;
+ }
+}120-120: Verify the spacing between collapse items
The margin uses CSS variables for consistency, but please verify that this spacing aligns with the design system's specifications.
✅ Verification successful
Based on the search results, I can now generate the final response:
The spacing values are correctly aligned with the design system
The margin values used in the collapse component (--te-common-vertical-item-spacing-normal: 12px and --te-common-vertical-form-label-spacing: 8px) are consistent with the design system's specifications as defined in packages/theme/base/src/common.less. These spacing variables are widely used across multiple components in the codebase for similar spacing patterns:
- Form elements spacing (12px)
- Title and list spacing (12px)
- Label to form element spacing (8px)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of these spacing variables
rg "te-common-vertical-item-spacing-normal|te-common-vertical-form-label-spacing" -A 2 -B 2
Length of output: 10520
267-279: 🛠️ Refactor suggestion
Enhance dialog close button accessibility
While the positioning is good, consider adding hover background and focus states for better accessibility.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-icon-secondary);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ padding: 4px;
+ border-radius: var(--te-base-border-radius-1);
&:hover {
color: var(--te-common-icon-primary);
+ background-color: var(--te-common-bg-container);
}
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/theme/base/src/component-common.less (2)
Line range hint
226-259: Consider enhancing keyboard focus stylesThe focus state is properly implemented, but consider adding a visible outline for keyboard navigation to improve accessibility.
.focus { border-color: var(--te-common-border-active); + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; }
752-763: Consider consolidating duplicate padding rulesThe padding rules are duplicated across multiple selectors. Consider consolidating them for better maintainability.
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label { - padding-left: 0; -} -.tiny-form--label-left.label-align - .tiny-form-item.is-required - ~ .tiny-form-item - .tiny-form-item__label.tiny-form-item__label { - padding-left: 0; -} -.tiny-form-item__label.tiny-form-item__label { - padding-right: 16px; -} +.tiny-form--label-left.label-align { + .tiny-form-item__label { + padding-left: 0; + padding-right: 16px; + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/common/component/PluginBlockList.vue(6 hunks)packages/theme/base/src/component-common.less(15 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/common/component/PluginBlockList.vue
🔇 Additional comments (8)
packages/theme/base/src/component-common.less (8)
120-120: LGTM: Consistent use of design system variables
The margin values properly utilize design system variables, ensuring consistency across the UI.
Line range hint 312-368: LGTM: Comprehensive button styling improvements
The changes properly handle button alignment, states, and icon spacing. Good use of design system variables for colors and spacing.
Line range hint 408-444: LGTM: Improved modal typography and spacing
Good use of design system variables for typography and spacing, creating clear visual hierarchy.
451-453: LGTM: Consistent button sizing
The minimum width ensures consistent button sizes in modal footers.
726-729: LGTM: Consistent form spacing
Good handling of form item margins with proper reset for last items.
Also applies to: 739-743
Line range hint 766-853: LGTM: Comprehensive grid styling
Good use of design system variables and consistent styling across grid components.
903-920: LGTM: Consistent textarea styling
Good implementation of states (hover, focus, disabled) and proper use of design system variables.
267-279: 🛠️ Refactor suggestion
Enhance dialog close button accessibility
While the styling is good, consider adding focus styles and aria-label for screen readers.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-icon-secondary);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
&:hover {
color: var(--te-common-icon-primary);
}
}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
packages/theme/base/src/component-common.less (4)
233-235: Consider enhancing keyboard navigation accessibilityWhile the focus state changes the border color, consider adding a visible outline for better keyboard navigation accessibility.
.focus { border-color: var(--te-common-border-active); + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; }
272-279: Enhance close button accessibilityConsider increasing the click target size and adding hover background for better usability.
.tiny-dialog-box__close { font-size: var(--ti-common-font-size-2); color: var(--te-common-icon-secondary); position: absolute; top: -4px; right: -4px; + padding: 4px; + cursor: pointer; &:hover { color: var(--te-common-icon-primary); + background-color: var(--te-common-bg-container); + border-radius: var(--te-base-border-radius-1); } }
839-846: Add hover transition for smoother interactionConsider adding a transition for the hover background color change.
&:hover { background-color: var(--te-common-bg-container); + transition: background-color 0.2s ease; }
907-923: Consider adding resize handle stylingThe textarea implementation should consider styling the resize handle for consistency.
.tiny-textarea.tiny-textarea { .tiny-textarea__inner { border-radius: var(--te-base-border-radius-1); border-color: var(--te-common-border-default); color: var(--te-common-text-primary); + resize: vertical; + &::-webkit-resizer { + border-width: 8px; + border-style: solid; + border-color: transparent var(--te-common-border-default) var(--te-common-border-default) transparent; + } &:hover { border-color: var(--te-common-border-active); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/theme/base/src/component-common.less(14 hunks)
🔇 Additional comments (4)
packages/theme/base/src/component-common.less (4)
120-120: LGTM!
The margin adjustment using CSS variables maintains consistency with the design system.
312-314: LGTM!
The flex layout improves alignment consistency, and the disabled state styling is well-defined.
Also applies to: 352-358
417-419: LGTM!
The typography changes maintain consistency with the design system.
Also applies to: 440-444
752-763: Simplify form label alignment rules
The current implementation has overlapping selectors that could be simplified.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/theme/base/src/component-common.less (2)
233-235: Enhance focus state accessibilityConsider adding an outline for better keyboard navigation visibility.
.focus { border-color: var(--te-common-border-active); + outline: 2px solid var(--te-common-border-active); + outline-offset: 1px; }
Line range hint
769-859: Consider reducing selector specificityThe triple class repetition
.tiny-grid__wrapper.tiny-grid__wrapper.tiny-grid__wrapperseems unnecessary and increases specificity.-.tiny-grid__wrapper.tiny-grid__wrapper.tiny-grid__wrapper { +.tiny-grid__wrapper {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/theme/base/src/component-common.less(14 hunks)
🔇 Additional comments (6)
packages/theme/base/src/component-common.less (6)
120-120: LGTM: Consistent use of spacing variables
The margin adjustment using CSS variables maintains consistency with the design system.
Line range hint 312-374: LGTM: Comprehensive button styling implementation
The button styles are well-structured with proper:
- Use of design system variables
- Consistent icon spacing
- Comprehensive state handling (hover, disabled)
Line range hint 414-450: LGTM: Well-structured modal styling
Good use of semantic variables and consistent spacing. The implementation aligns well with the PR objectives for improving modal appearance.
755-766: Form label alignment implementation matches previous recommendation
The implementation follows the previous review suggestion for simplifying form label alignment rules.
909-926: LGTM: Consistent textarea implementation
The textarea styles maintain consistency with input styles and properly handle all states using design system variables.
272-279: Consider mobile-friendly close button positioning
The absolute positioning with negative values might cause the close button to be cut off on small screens. Consider using a more responsive approach.
✅ Verification successful
Close button positioning appears safe with container padding
The dialog box has a 20px padding which provides sufficient space for the close button's negative positioning (-4px) without risk of overflow or cutoff. The absolute positioning is relative to the padded container, making it a safe approach even on mobile devices.
- The close button is positioned within
.tiny-dialog-box__headerwhich is part of.tiny-dialog-boxthat haspadding: 20px - The -4px offset from
topandrightstill keeps the button well within the padded area - No specific mobile breakpoints or issues were found that would affect this positioning
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any mobile-specific styles for the close button
rg -A 2 "tiny-dialog-box__close.*@media.*mobile"
Length of output: 50
Script:
#!/bin/bash
# Let's check for any responsive/mobile styles for dialog box or close button
rg -A 5 "@media.*\{.*dialog" packages/theme
# Check for any existing mobile breakpoint definitions
rg "@tiny-mobile|@mobile|--tiny-mobile|--mobile" packages/theme
# Check for any dialog close button styles that might handle responsive behavior
rg -A 3 "dialog.*close|tiny-dialog.*close" packages/theme
Length of output: 504
Script:
#!/bin/bash
# Let's check the complete dialog box component styles to understand the container's positioning context
rg -B 5 -A 10 "tiny-dialog-box\s*\{" packages/theme
# Check for any existing dialog box size definitions or constraints
rg "dialog.*width|dialog.*height|dialog.*size" packages/theme
# Look for any existing responsive utilities or mixins that might be relevant
fd . packages/theme -e less -x cat {} | rg -A 3 "@media.*\{|@screen"
Length of output: 1340
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/theme/base/src/component-common.less (2)
Line range hint
229-262: Consider enhancing keyboard focus visibilityWhile the focus state is implemented well, consider adding a visible outline for keyboard navigation to improve accessibility.
.focus { border-color: var(--te-common-border-active); + &:focus-visible { + outline: 2px solid var(--te-common-border-active); + outline-offset: 2px; + } }
Line range hint
772-862: Add keyboard focus states for grid rowsWhile hover states are implemented, consider adding keyboard focus states for better accessibility.
.tiny-grid-body__row { &:hover { background-color: var(--te-common-bg-container); } + &:focus-visible { + outline: 2px solid var(--te-common-border-active); + outline-offset: -2px; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/plugins/state/src/CreateVariable.vue(3 hunks)packages/theme/base/src/component-common.less(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/state/src/CreateVariable.vue
🔇 Additional comments (6)
packages/theme/base/src/component-common.less (6)
123-123: LGTM! Good use of semantic spacing variables.
The margin adjustment using semantic variables improves maintainability and consistency.
Line range hint 315-377: LGTM! Good accessibility improvements
The button styles are well-implemented with:
- Proper disabled states with semantic colors
- Consistent icon spacing
- Clear hover states
Line range hint 417-453: Apply similar accessibility improvements as dialog close button
The modal close button should have similar accessibility improvements as suggested for the dialog close button.
912-929: LGTM! Consistent with input styles
The textarea implementation maintains consistency with input styles and includes proper hover, focus, and disabled states.
758-769: 🛠️ Refactor suggestion
Simplify form label alignment rules
The current implementation has multiple overlapping selectors for label alignment. This could be simplified to reduce specificity and improve maintainability.
-.tiny-form--label-left.label-align .tiny-form-item.is-required .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form--label-left.label-align
- .tiny-form-item.is-required
- ~ .tiny-form-item
- .tiny-form-item__label.tiny-form-item__label {
- padding-left: 0;
-}
-.tiny-form-item__label.tiny-form-item__label {
- padding-right: 16px;
-}
+.tiny-form--label-left.label-align {
+ .tiny-form-item__label {
+ padding-left: 0;
+ padding-right: 16px;
+ }
+}Likely invalid or redundant comment.
270-294: 🛠️ Refactor suggestion
Enhance dialog close button accessibility
The close button positioning is good, but consider adding hover feedback and keyboard focus styles for better accessibility.
.tiny-dialog-box__close {
font-size: var(--ti-common-font-size-2);
color: var(--te-common-icon-secondary);
position: absolute;
top: -4px;
right: -4px;
+ cursor: pointer;
+ padding: 4px;
+ border-radius: var(--te-base-border-radius-1);
&:hover {
color: var(--te-common-icon-primary);
+ background-color: var(--te-common-bg-container);
}
+ &:focus-visible {
+ outline: 2px solid var(--te-common-border-active);
+ outline-offset: 2px;
+ }
}Likely invalid or redundant comment.
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
1.全局弹窗（标题问题，关闭按钮样式，对齐问题，按钮间距，表单间距，标题与内容间距）
2.全局搜索（图标样式和间距，搜索激活态描边，提示文本，搜索组件间距）
3.组件列表间距调整
4.选块组件（页签）灰背景圆角
5.区块管理-区块列表（列表样式，底部筛选样式和底部背景）
6.按钮样式调整
7.状态管理/资源管理代码示例背景，字体调整
8.页面schema分割线和面板阴影调整
9.国际化页面样式调整
10.左侧菜单图标样式调整
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
Release Notes
New Features
Bug Fixes
Style
Refactor
These updates enhance usability, improve visual consistency, and refine user interactions across the application.