Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(styles): apply new styles to props panel #830

Merged

Conversation

gene9831
Copy link
Collaborator

@gene9831 gene9831 commented Sep 29, 2024

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

属性面板改动点:

  1. 字体从14px改为12px
  2. 属性label位置规则修改:
    • 如果label较短,则label和简单输入组件为左右结果,处于同一行
      {7651EF07-DB54-4268-9589-BFAF0E0D7A7C}
    • 如果label较长,则label和简单输入组件为上下结构,分两行展示
      {A92E4D17-A33E-4612-B7A7-A05AD94F4C1A}
    • 如果是编辑代码组件,统一为上下结构
      {2831DA36-5D83-463D-9E56-D2CD15407E64}
    • 如果是复选框组件,统一为左右结构
      {C60B62D1-D7A8-4833-8A4C-9FD69BDC20A1}
  3. 只有少数几个选项的选择器,修改为选块(目前只修改了仅有2个选项并且文本较短的选择器)
    {45B5ADEB-8CBD-429F-98AF-2C9BDEE6CE72}
  4. 表格列"新增一列"修改位置和样式
    {0E720DF3-3BEC-4680-9762-A4A22214DD75}
  5. 表格二级面板增加关联标题(标题为空展示为"未命名标题")
    image
  6. 样式面板边框属性样式修改
    Before:
    {388A47F3-91BC-403B-8BE1-D3BF2B5DE589}
    After:
    {9AB14B5F-3CFA-440D-B3C4-2522423EC89C}

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

Release Notes

  • New Features

    • Standardized label positions for form elements to enhance visual consistency.
    • Introduced new properties for input fields, including maxlength, autofocus, and clearable.
    • Added new components: TinyCarouselItem, TinyCarousel, and ButtonGroup for enhanced button interactions.
    • Implemented a new title display in MetaChildItem for improved UI clarity.
    • Added a dedicated button for adding items in ArrayItemConfigurator.
    • Redesigned border and radius selection interface for improved usability.
  • Bug Fixes

    • Enhanced event handling and styling for various components, improving overall functionality and user experience.
  • Style

    • Comprehensive updates to styling across multiple components to improve visual consistency and responsiveness.

Copy link
Contributor

coderabbitai bot commented Sep 29, 2024

Walkthrough

The pull request introduces a series of modifications across multiple components in a Vue.js application, focusing on configuration and schema definitions for UI elements. Key changes include adjustments to label positions, updates to widget components, and the introduction of new properties for enhanced functionality. Additionally, several components have undergone structural and stylistic refinements, improving code readability and visual consistency throughout the application.

Changes

File Path Change Summary
designer-demo/public/mock/bundle.json Adjusted label positions, replaced SwitchConfigurator with CheckBoxConfigurator, and added new properties.
packages/common/component/ConfigItem.vue Simplified template structure and improved code readability; no public entity changes.
packages/common/component/MetaChildItem.vue Added a new computed property title for enhanced UI; no public entity changes.
packages/common/component/MetaCodeEditor.vue Updated template structure and styles; no public entity changes.
packages/common/component/MetaList.vue Introduced a new bottom slot for flexible content placement; no public entity changes.
packages/common/component/MetaListItem.vue Modified event handling and styling; no public entity changes.
packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue Removed actions template, added bottom template for item addition.
packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue Updated template and styles for button group; no public entity changes.
packages/configurator/src/checkbox-configurator/CheckBoxConfigurator.vue Simplified template structure and introduced change method; added inheritAttrs property.
packages/plugins/materials/src/js/options.js Added labelPosition attribute to properties in basePropertyOptions.
packages/theme/base/src/component-common.less Extensive styling adjustments across various UI components to enhance visual presentation and user interaction.
packages/settings/styles/src/components/border/BorderGroup.vue Restructured template and script for border and radius settings; added computed properties.
packages/settings/styles/src/components/buttons/ButtonGroup.vue Introduced a new button group component with tooltips and dynamic rendering.
packages/settings/styles/src/components/index.js Added export statement for the new ButtonGroup component.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • hexqi
  • chilingling

Poem

🐇 In the garden where changes bloom,
New labels dance, dispelling gloom.
Buttons and checkboxes, side by side,
With every tweak, our joy will glide.
A hop, a skip, a leap so bright,
UI shines, oh what a sight! 🌼


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d29a3f7 and fa7160e.

📒 Files selected for processing (1)
  • packages/common/component/MetaChildItem.vue (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/common/component/MetaChildItem.vue

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the refactor-main refactor/develop branch feature label Sep 29, 2024
@gene9831 gene9831 marked this pull request as ready for review September 30, 2024 09:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (12)
packages/common/component/MetaList.vue (2)

14-16: LGTM! Consider adding a comment for clarity.

The addition of the "bottom" slot enhances the component's flexibility. The placement and naming are consistent with the existing structure.

Consider adding a comment to explain the purpose of this new slot:

 <div class="bottom">
+  <!-- Slot for additional content at the bottom of the list -->
   <slot name="bottom"></slot>
 </div>

38-43: LGTM! Consider using CSS variables for consistency.

The addition of styles for the new "bottom" class is appropriate and well-structured. The use of flexbox is a good choice for layout management.

For consistency with the existing code and to improve maintainability, consider using CSS variables for the padding and margin values:

 .bottom {
   display: flex;
-  padding: 2px;
-  margin-top: 4px;
+  padding: var(--ti-lowcode-meta-list-bottom-padding, 2px);
+  margin-top: var(--ti-lowcode-meta-list-bottom-margin-top, 4px);
 }

This change allows for easier theming and consistent styling across the application.

packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue (1)

62-94: LGTM: Improved button group styling with a minor suggestion

The style changes enhance the button group's responsiveness and visual consistency. The use of flex layout and custom styling for first/last items improves the overall appearance.

Consider using CSS custom properties (variables) for values like colors and dimensions to improve maintainability. For example:

:root {
  --button-height: 24px;
  --button-border-radius: 4px;
  /* Add more variables as needed */
}

/* Then use them in your styles */
li button {
  height: var(--button-height);
  border-radius: var(--button-border-radius);
  /* ... */
}

This approach would make it easier to maintain consistent values across the component and potentially the entire application.

packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (2)

41-46: LGTM! Consider internationalization for the "Add a column" text.

The new #bottom template successfully implements the "Add a Column" option as described in the PR objectives. The change improves the UI by providing a clear, clickable element for adding new items.

Consider using an internationalization method for the text "新增一列" to support multiple languages. For example:

- <span>新增一列</span>
+ <span>{{ translate('addColumn') }}</span>

Ensure to add the corresponding translation key in your localization files.


197-209: LGTM! Consider using consistent units for measurements.

The new styles for the .add class effectively implement the visual changes for the "Add a Column" option as described in the PR objectives. The use of flexbox and hover effects improves the user experience.

For consistency, consider using the same unit (px) for all measurements:

.add {
  /* ... existing styles ... */

  & .icon-plus {
    font-size: 14px;
-   margin-right: 5px;
+   margin-right: 5px;
  }
}

This small change ensures consistency in the codebase and makes it easier to maintain and scale styles in the future.

packages/common/component/MetaListItem.vue (1)

Line range hint 1-324: Suggestions for code improvement

While the changes look good, here are some suggestions for further improvement:

  1. Consider extracting the logic for handling different button actions in the btnClick function into separate methods for better code organization and testability.

  2. The component mixes English and non-English comments and strings. For better internationalization, consider using a translation system (e.g., vue-i18n) to handle non-English text.

  3. The component seems to handle quite a few responsibilities. Consider if some of its functionality could be split into smaller, more focused components to improve maintainability.

packages/common/component/MetaCodeEditor.vue (1)

Line range hint 1-365: Consider full migration to Composition API for consistency.

The component currently uses a mix of Composition API (in the setup function) and Options API (for component definition). While this doesn't cause any immediate issues, consider fully migrating to Composition API for better consistency and to leverage its benefits throughout the component.

This refactoring would involve:

  1. Moving all component logic into the setup function.
  2. Using defineProps and defineEmits for prop and emit definitions.
  3. Utilizing <script setup> syntax for a more concise component structure.

Here's a high-level example of how the script section could be refactored:

<script setup>
import { computed, nextTick, reactive, ref, watchEffect } from 'vue'
import { Button, DialogBox, Split } from '@opentiny/vue'
import VueMonaco from './VueMonaco.vue'
import { formatString } from '../js/ast'
import i18n from '../js/i18n'

const props = defineProps({
  buttonText: {
    type: [String, Object],
    default: '编辑代码'
  },
  // ... other props
})

const emit = defineEmits(['save', 'open', 'update:modelValue', 'close'])

const { locale } = i18n.global
const editorState = reactive({
  // ... existing editorState properties
})
const value = ref('')
const editor = ref(null)

const buttonLabel = computed(() => props.buttonText?.[locale.value] ?? props.buttonText)
// ... other computed properties

// ... existing methods (close, open, parseContent, etc.)

// Expose necessary properties and methods
defineExpose({
  save,
  close,
  open,
  formatCode,
  editorDidMount,
  editor,
  editorState,
  value,
  options: {
    language: props.language,
    minimap: {
      enabled: false
    }
  }
})
</script>

This refactoring would make the component more consistent with modern Vue 3 practices and potentially easier to maintain.

packages/common/component/ConfigItem.vue (1)

Line range hint 1-687: Suggestions for future refactoring

While the current changes are minimal and don't introduce any issues, here are some suggestions for future improvements:

  1. Consider breaking down this large component into smaller, more manageable sub-components to improve maintainability.
  2. The verifyValue method is quite complex. Consider refactoring it into smaller, more focused functions for better readability and easier testing.
  3. The use of generateFunction to execute dynamic code could pose security risks. Ensure that all inputs are properly sanitized and consider alternative approaches if possible.

These suggestions are not critical for the current PR but could be considered for future refactoring efforts.

designer-demo/public/mock/bundle.json (4)

Line range hint 14-12774: Inconsistent property naming in component definitions

There are inconsistencies in property naming across different component definitions. For example, some components use "dev_mode" while others use "devMode". It's recommended to standardize the naming convention throughout the file for better maintainability.

-  "dev_mode": "proCode",
+  "devMode": "proCode",

Consider alternatives to CDN scripts in npm configuration

Some components include CDN script URLs in their npm configuration. While this can be convenient, it may not be suitable for all use cases, especially in production environments. Consider providing options for both CDN and local package management.

Expand configure sections for better customization

Several components have empty or minimal "configure" sections. To improve the flexibility and usability of these components, consider expanding these sections with more customization options where appropriate.


Line range hint 12775-14089: Consider parameterizing hardcoded values in snippets

Some snippets contain hardcoded values, particularly in style attributes. To improve flexibility and reusability, consider parameterizing these values or providing options for customization.

Example:

- "style": "margin:10px 0 0 30px"
+ "style": { "margin": "{{ topMargin || '10px' }} 0 0 {{ leftMargin || '30px' }}" }

Add documentation for complex snippets

For more complex snippets with nested components or intricate structures, consider adding inline documentation or comments to explain the purpose and usage of different parts. This will help users understand and customize these snippets more effectively.

Standardize property naming in snippets

There are some inconsistencies in property naming across different snippets. Standardizing these names would improve the overall coherence of the snippet library. For example, ensure consistent use of camelCase or kebab-case for multi-word property names.


Line range hint 1-14089: Consider splitting the configuration file for better performance

The current file is quite large, which could lead to performance issues when loading or parsing the configuration. Consider splitting this into multiple smaller files, perhaps organized by component type or functionality. This would improve load times and make the configuration more manageable.

Standardize naming conventions throughout the file

There are inconsistencies in naming conventions used across the file. Standardize on either camelCase or snake_case for property names, and ensure consistent naming patterns for similar concepts across different components and snippets.

Improve documentation consistency

The level of documentation varies across different sections of the file. Aim to provide consistent and comprehensive documentation for all components, properties, and snippets. This will greatly improve the usability of the design system.

Add version control for configuration structure

Given the comprehensive nature of this configuration file, any structural changes could potentially cause breaking changes for consumers. Consider implementing a versioning system for the configuration structure, allowing for backwards compatibility and smoother updates.


Line range hint 1-14089: Summary and Recommendations

The bundle.json file provides a comprehensive configuration for the design system, including detailed component definitions and useful snippets. However, several areas for improvement have been identified:

  1. Inconsistent naming conventions across components and properties.
  2. Potential performance issues due to the large file size.
  3. Varying levels of documentation completeness.
  4. Some hardcoded values in snippets that could be more flexible.

Recommendations:

  1. Standardize naming conventions throughout the file.
  2. Consider splitting the configuration into multiple files for better performance and maintainability.
  3. Improve and standardize documentation across all sections.
  4. Parameterize hardcoded values in snippets for greater flexibility.
  5. Implement a versioning system for the configuration structure.

Addressing these issues will significantly improve the maintainability, performance, and usability of the design system configuration.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 046dca4 and 784d878.

📒 Files selected for processing (11)
  • designer-demo/public/mock/bundle.json (203 hunks)
  • packages/common/component/ConfigItem.vue (7 hunks)
  • packages/common/component/MetaChildItem.vue (4 hunks)
  • packages/common/component/MetaCodeEditor.vue (4 hunks)
  • packages/common/component/MetaList.vue (2 hunks)
  • packages/common/component/MetaListItem.vue (5 hunks)
  • packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (2 hunks)
  • packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue (2 hunks)
  • packages/configurator/src/checkbox-configurator/CheckBoxConfigurator.vue (2 hunks)
  • packages/plugins/materials/src/js/options.js (3 hunks)
  • packages/theme/base/src/component-common.less (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/theme/base/src/component-common.less
🔇 Additional comments (28)
packages/configurator/src/checkbox-configurator/CheckBoxConfigurator.vue (5)

2-2: LGTM! Template simplification improves readability.

The template has been simplified, which improves readability. The use of v-model for two-way binding is correct. The addition of the @change event handler is noted and will be verified in the script section.


20-22: Good use of watchEffect for simplified reactivity.

The switch from watch to watchEffect simplifies the reactivity logic. This approach is more concise and automatically tracks its reactive dependencies.


24-26: LGTM! The change method correctly handles updates.

The new change method correctly emits the updated value using the update:modelValue event. This implementation properly supports the v-model directive used in the template.


16-16: Good practice: Added inheritAttrs: false.

Adding inheritAttrs: false prevents unwanted attribute inheritance, which is a good practice for components that manage their own attributes.


Line range hint 1-31: Overall, excellent refactoring that improves component quality.

The changes in this file represent a well-executed refactoring of the CheckBoxConfigurator component. The template has been simplified, and the script section now uses more modern Vue.js practices (watchEffect, v-model with custom components). The addition of inheritAttrs: false shows attention to detail in preventing unintended attribute inheritance.

These changes improve the component's readability, maintainability, and align well with Vue.js best practices. The functionality appears to be maintained while the code quality has been enhanced.

packages/common/component/MetaList.vue (1)

Line range hint 1-43: Overall, the changes enhance component flexibility while maintaining consistency.

The modifications to MetaList.vue align well with the PR objectives. The addition of the "bottom" slot and corresponding styles improves the component's versatility without introducing functional changes or breaking existing behavior. The code structure and naming conventions remain consistent with the existing implementation.

To further improve the component:

  1. Consider adding a comment to explain the purpose of the new "bottom" slot.
  2. Consider using CSS variables for the new style properties to enhance maintainability and theming capabilities.

These suggestions are minor and do not impact the overall quality of the changes.

packages/plugins/materials/src/js/options.js (4)

23-24: LGTM: Addition of labelPosition aligns with PR objectives.

The addition of labelPosition: 'left' to the id property is consistent with the PR's goal of updating positioning rules for property labels. This change allows short labels to align horizontally with simple input components, as described in the PR objectives.


40-41: LGTM: Consistent application of labelPosition.

The addition of labelPosition: 'left' to the className property maintains consistency with the id property and aligns with the PR objectives. This change contributes to a uniform styling approach for similar properties.


57-58: LGTM: Completes consistent styling across properties.

The addition of labelPosition: 'left' to the ref property completes the consistent application of this styling rule across all properties in the group (id, className, and ref). This change maintains the uniform approach to label positioning as outlined in the PR objectives.


23-24: Overall changes align with PR objectives, but may be incomplete.

The consistent addition of labelPosition: 'left' to the id, className, and ref properties aligns well with the PR objectives of updating positioning rules for property labels. These changes contribute to the goal of having short labels align horizontally with simple input components.

However, these modifications only address a small part of the described style updates in the PR objectives. To ensure complete implementation of the PR goals:

  1. Verify that similar changes have been made in other relevant files or components.
  2. Check if the other style adjustments mentioned in the PR objectives (e.g., font size reduction, vertical stacking for long labels, block selection for selectors with few options) have been implemented elsewhere.

To help verify the completeness of the changes across the codebase, you can run the following script:

This script will help identify if similar changes have been made consistently across the codebase and if other aspects of the PR objectives have been implemented.

Also applies to: 40-41, 57-58

packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue (2)

7-7: LGTM: Improved template formatting

The reformatting of the @update:modelValue event listener improves code readability without affecting functionality.


Line range hint 1-101: Summary: Successful style refactoring with improved visual consistency

The changes in this file successfully refactor the styles of the ButtonGroupConfigurator component, aligning with the PR objectives. The modifications enhance the visual consistency and responsiveness of the button group without introducing functional changes.

Key improvements:

  1. Reformatted template for better readability
  2. Enhanced button group layout using flex
  3. Improved styling for active and inactive buttons
  4. Adjusted button dimensions for better visual appeal

These changes contribute to a more polished and consistent user interface. The suggestion to use CSS variables could further improve maintainability in the future.

packages/common/component/MetaChildItem.vue (4)

3-3: LGTM: Title addition enhances UI clarity

The addition of the title element improves the user interface by providing a clear header for the list of properties. This change aligns well with the PR objectives of updating the properties panel styles.


57-59: LGTM: Improved handling of array types

The modification to the properties computed property enhances the component's flexibility by correctly handling both object and array types. This change improves the overall robustness of the component.


82-86: LGTM: Appropriate styling for the new title element

The added styles for the title element are appropriate and consistent with the PR objectives. The use of CSS variables for colors is a good practice for maintaining a consistent design system.


Line range hint 1-99: Summary: Well-implemented changes with a minor suggestion for improvement

Overall, the changes to MetaChildItem.vue are well-implemented and align with the PR objectives. The addition of the title element, the new computed property for determining the title, and the improved handling of array types in the properties computed property all contribute to enhancing the component's functionality and UI.

The only suggestion for improvement is to consider a more robust internationalization approach for the title fallback logic.

Great job on these refactoring changes!

packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (1)

Line range hint 1-209: Summary: Changes successfully implement PR objectives

The modifications in this file effectively implement the changes described in the PR objectives, particularly:

  1. Altering the position and style of the "Add a Column" option in tables.
  2. Removing the MetaListActions component and replacing it with a more streamlined UI element.

The changes are consistent with the AI-generated summary and maintain the existing functionality while improving the user interface.

To further enhance the code:

  1. Consider internationalizing the "Add a column" text for better language support.
  2. Remove unused code related to the old MetaListActions implementation.
  3. Ensure consistent use of measurement units in the CSS.

These minor improvements will increase maintainability and consistency across the codebase.

packages/common/component/MetaListItem.vue (3)

21-21: Improved event handler syntax in template

The changes to the event handlers in the template are good improvements. By using the closing tag syntax (e.g., @hide="hide(item)" instead of :@hide="hide"), the code becomes more explicit and consistent with Vue.js best practices. This approach can help prevent potential issues with event binding and makes the code more readable.

Also applies to: 31-31, 49-49, 67-67


Line range hint 78-260: No changes in the script section

The script section doesn't show any significant changes that require review.


262-262: Style adjustments align with PR objectives

The style changes made to .active-item and .option-input are in line with the PR objectives for refactoring the properties panel styles. The new background color for active items and the defined height with vertical alignment for option inputs should improve the visual consistency and layout of the panel.

Also applies to: 266-267

packages/common/component/MetaCodeEditor.vue (4)

19-19: LGTM: Template changes improve readability and consistency.

The changes in the template section are minor but positive:

  1. Reformatting of :close-on-click-modal improves code readability.
  2. Adding @editorDidMount event handler is consistent with the component's setup function.
  3. Moving @click to a new line is a minor formatting improvement.

These changes enhance the overall code quality without affecting functionality.

Also applies to: 32-32, 53-53


256-257: Approved: Use of CSS variables enhances theme consistency.

The update to use CSS variables for color and border-color in the .edit-btn class is a positive change. This modification:

  1. Improves theme consistency across the application.
  2. Makes it easier to manage and update styles globally.
  3. Aligns with the PR objectives of refactoring styles.

These changes contribute to a more maintainable and flexible styling system.


262-265: Approved: Consistent use of CSS variables for button states.

The update to use CSS variables for border-color in the hover and focus states of .edit-btn is a good improvement:

  1. Ensures consistent styling across different button states.
  2. Aligns with the previous changes, maintaining a uniform approach to styling.
  3. Contributes to the overall goal of refactoring styles as mentioned in the PR objectives.

These changes enhance the maintainability and consistency of the component's styling.


Line range hint 1-365: Verify completeness of style changes across the project.

While the changes in this file align with the PR objectives of refactoring styles without introducing functional changes, some of the mentioned style updates are not present in this file:

  1. The font size reduction from 14px to 12px is not visible here.
  2. The positioning rules for property labels (short labels aligning horizontally, long labels stacking vertically) are not implemented in this file.
  3. Changes to selectors and the "Add a Column" option in tables are not present.

To ensure all mentioned changes are implemented, please run the following script:

This script will help verify if the mentioned changes are implemented in other files of the project.

packages/common/component/ConfigItem.vue (3)

11-11: Formatting improvements approved

The changes in the template section are minor formatting adjustments that improve code readability without affecting functionality. These changes align with best practices for Vue template formatting.

Also applies to: 20-20, 44-44, 60-60, 78-78, 90-90


518-518: Font size reduction approved

The font size reduction from 14px to 12px for the .item-label class aligns with the PR objectives. This change contributes to the overall style refinement of the properties panel.


Line range hint 1-687: Final approval: Changes align with PR objectives

The modifications in this file are minimal and align well with the PR objectives:

  1. The font size reduction from 14px to 12px for property labels has been implemented.
  2. Minor formatting improvements have been made to enhance code readability.

These changes do not introduce any functional modifications or potential issues. The overall structure and logic of the component remain intact. The file changes are approved and ready for merging.

designer-demo/public/mock/bundle.json (1)

Line range hint 1-13: LGTM: Overall structure is well-organized

The JSON file has a logical and well-structured layout, with clear separation between different sections such as "framework", "materials", and "snippets". This organization makes it easier to navigate and maintain the configuration.

@gene9831 gene9831 added the ready_for_review This PR requires more reviews label Sep 30, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (4)
packages/settings/styles/src/components/buttons/ButtonGroup.vue (3)

1-12: LGTM! Consider enhancing accessibility.

The template structure is well-organized and follows Vue.js best practices. The use of tiny-tooltip for each option enhances the user experience, and the conditional rendering of icons and labels provides flexibility.

Consider adding aria-label attributes to the <li> elements to improve accessibility, especially for screen readers. For example:

- <li :data-active="item.value === value ? '' : undefined" @click="handleClick(item.value)">
+ <li :data-active="item.value === value ? '' : undefined" @click="handleClick(item.value)" :aria-label="item.label || item.tip">

14-41: LGTM! Consider enhancing type safety.

The script section is well-structured and follows Vue 3 Composition API best practices. Props are properly defined, and the watcher ensures the internal state stays in sync with the prop. The handleClick method correctly updates the state and emits an event.

To improve type safety and developer experience, consider using TypeScript or adding JSDoc type annotations. For example:

interface ButtonOption {
  value: string;
  label?: string;
  icon?: string;
  tip: string;
}

const props = defineProps<{
  options: ButtonOption[];
  modelValue: string;
}>();

This will provide better type checking and autocompletion in IDEs.


43-90: LGTM! Consider performance optimization for hover effects.

The style section is well-structured, using scoped LESS styles and CSS variables for theming. The layout is responsive, and the styling for active states and separators enhances the user experience.

To potentially improve performance, consider using CSS will-change property for hover effects. This can help the browser optimize rendering, especially on lower-end devices. Add the following to the li selector:

li {
  // ... existing styles ...
  will-change: background-color, color;
}

Note: Use will-change sparingly, as overuse can lead to performance issues. In this case, it's appropriate due to the frequent state changes in a button group.

packages/settings/styles/src/components/border/BorderGroup.vue (1)

88-95: Ensure consistent usage of class bindings and event handlers

In the <span> element for the border label, there is a mix of static and dynamic class bindings. Additionally, the event handler might need verification to ensure it functions as expected.

Consider adjusting the class binding syntax for consistency:

-<span
-  class="border-label"
-  :class="{ 'is-setting': isBorderSetting(), 'set-border-style': true }"
+<span
+  :class="['border-label', 'set-border-style', { 'is-setting': isBorderSetting() }]"
   @click="openSetting(BORDER_PROPERTY.Border, $event)"
 >
   边框
 </span>

This change consolidates the class bindings into a single :class array, enhancing readability.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 784d878 and 74e5d0f.

⛔ Files ignored due to path filters (5)
  • packages/design-core/assets/border-all.svg is excluded by !**/*.svg
  • packages/design-core/assets/border-bottom.svg is excluded by !**/*.svg
  • packages/design-core/assets/border-left.svg is excluded by !**/*.svg
  • packages/design-core/assets/border-right.svg is excluded by !**/*.svg
  • packages/design-core/assets/border-top.svg is excluded by !**/*.svg
📒 Files selected for processing (5)
  • packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue (2 hunks)
  • packages/configurator/src/color-configurator/ColorConfigurator.vue (0 hunks)
  • packages/settings/styles/src/components/border/BorderGroup.vue (10 hunks)
  • packages/settings/styles/src/components/buttons/ButtonGroup.vue (1 hunks)
  • packages/settings/styles/src/components/index.js (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/configurator/src/color-configurator/ColorConfigurator.vue
🧰 Additional context used
🔇 Additional comments (12)
packages/settings/styles/src/components/index.js (1)

15-15: LGTM! New ButtonGroup component export added correctly.

The export statement for the ButtonGroup component is correctly formatted and placed in alphabetical order. This addition makes the new component available for use in other parts of the application.

To ensure the ButtonGroup component file exists, run the following command:

✅ Verification successful

Verified! ButtonGroup.vue file exists as expected.

The ButtonGroup.vue file has been successfully located in the specified directory, confirming the correctness of the export statement.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the ButtonGroup.vue file
fd ButtonGroup.vue packages/settings/styles/src/components/buttons

Length of output: 132

packages/settings/styles/src/components/buttons/ButtonGroup.vue (1)

1-90: Overall assessment: Well-implemented component with minor improvement suggestions.

This new ButtonGroup component is well-structured and aligns with the PR objectives of refactoring styles without introducing functional changes. It contributes to improving the properties panel's appearance by providing a flexible and visually appealing button group implementation.

Key strengths:

  1. Follows Vue.js best practices and uses the Composition API effectively.
  2. Implements tooltips for enhanced user experience.
  3. Uses CSS variables for easy theming and consistency.
  4. Provides flexible rendering of icons and labels.

Suggestions for improvement:

  1. Enhance accessibility by adding aria-label attributes.
  2. Improve type safety with TypeScript or JSDoc annotations.
  3. Consider performance optimization for hover effects using will-change.

These minor improvements will further enhance the component's quality and maintainability.

packages/configurator/src/button-group-configurator/ButtonGroupConfigurator.vue (7)

8-9: Formatting adjustment improves readability

The closing tag for <tiny-button-group> has been moved to a new line, enhancing the readability of the template structure.


64-65: Ensure button group occupies full width

Adding width: 100% to .meta-button-group.tiny-button-group ensures the button group spans the full width of its container, providing consistent styling.


66-71: Implement flex layout for button items

Applying display: flex to ul.tiny-group-item and setting flex: 1 on li elements distributes buttons evenly, improving the layout responsiveness.


73-79: Adding border-radius to first and last buttons

This change rounds the corners of the first and last buttons, enhancing the visual appeal of the button group.


81-88: Update background and text colors using CSS variables

The use of new CSS variables for background and text colors promotes consistency and simplifies theme management.


90-99: Adjust button sizing and text overflow handling

Setting min-width, width, padding, and text overflow properties ensures buttons are sized appropriately and text is displayed correctly within the available space.


101-111: Implement separator between buttons

Adding the ::after pseudo-element to create visual separators between buttons enhances the user interface by clearly delineating individual buttons.

packages/settings/styles/src/components/border/BorderGroup.vue (3)

403-406: Fix the icon name for 'none' border style

In the styleOptions array, the icon for the 'none' border style is set as 'cross'. Verify if this icon represents the 'none' style appropriately.

If there is a specific icon that better represents the 'none' border style, consider updating it. For example:

 { icon: 'cross', value: BORDER_STYLE_TYPE.None, tip: 'none-无' },
+// Alternatively, use an icon named 'border-style-none' if available
+{ icon: 'border-style-none', value: BORDER_STYLE_TYPE.None, tip: 'none-无' },

582-587: Ensure depth selectors are applied correctly in Less

The use of :deep in Less files may not be necessary. Verify that the styles are being scoped correctly.

Check if the :deep selector is required in the following style:

.radius-content {
  display: flex;
  gap: 4px;

  // Is the :deep selector necessary here?
  .radius-btn-group:deep(ul) {
    width: 48px;
  }
}

If not required, you can remove :deep to simplify the code:

.radius-content {
  display: flex;
  gap: 4px;

  .radius-btn-group ul {
    width: 48px;
  }
}

193-193: Verify the import path for ButtonGroup component

The import statement for ButtonGroup and its registration in the components object should be checked to ensure correctness.

Run the following script to confirm the existence and correct path of the ButtonGroup component:

Also applies to: 231-231

✅ Verification successful

Verified the import path for ButtonGroup component

  • The ButtonGroup.vue component exists at packages/settings/styles/src/components/buttons/ButtonGroup.vue, which matches the import path ../buttons/ButtonGroup.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the ButtonGroup component exists at the specified path.

# Test: Check for the existence of ButtonGroup.vue or ButtonGroup/index.js
# Expected result: The files should be found in the specified directory.

# Search for ButtonGroup.vue
fd ButtonGroup.vue src/

# Search for index.js in ButtonGroup directory
fd index.js src/ --search-path 'buttons/ButtonGroup'

Length of output: 318


Script:

#!/bin/bash
# Description: Verify that the ButtonGroup component exists at the specified path.

# Test: Check for the existence of ButtonGroup.vue or ButtonGroup/index.js
# Expected result: The files should be found in the specified directory.

# Search for ButtonGroup.vue in the repository
fd ButtonGroup.vue .

# Search for index.js within the buttons/ButtonGroup directory
fd index.js ./buttons/ButtonGroup

Length of output: 277

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
packages/settings/styles/src/components/border/BorderGroup.vue (2)

Line range hint 609-611: Replace deprecated :deep selector with ::v-deep

The use of :deep in the .radius-content-input styles is deprecated. Update it to ::v-deep to maintain compatibility with the latest Vue standards.

Apply this diff to make the change:

 .radius-content-input {
   margin-left: 4px;

-  :deep(.tiny-input__suffix) {
+  ::v-deep .tiny-input__suffix {
     right: 0;
   }
 }

Line range hint 623-635: Replace deprecated :deep selector with ::v-deep

In the styles for .radius-content, the :deep combinator is deprecated. Update it to ::v-deep to follow Vue 3 best practices.

Here's the diff to update the selector:

 .radius-content {
   flex: 1;
   display: flex;
   align-items: center;
   margin: 5px 0;

-  :deep(.tiny-input__inner) {
+  ::v-deep .tiny-input__inner {
     background: var(--ti-lowcode-tabs-active-bg);
     padding: 0 20px 0 0;
     border-radius: 0;
     border-width: 0;
     &:hover,
     &:focus {
       border-bottom: 1px solid var(--ti-lowcode-property-active-color);
     }
   }
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 74e5d0f and 2877e00.

📒 Files selected for processing (1)
  • packages/settings/styles/src/components/border/BorderGroup.vue (10 hunks)
🧰 Additional context used
🔇 Additional comments (1)
packages/settings/styles/src/components/border/BorderGroup.vue (1)

585-587: ⚠️ Potential issue

Replace deprecated :deep selector with ::v-deep

The :deep selector is deprecated in Vue 3. To ensure future compatibility and adhere to best practices, use the ::v-deep combinator instead.

Apply the following diff to update the selector:

-.radius-btn-group:deep(ul) {
+.radius-btn-group ::v-deep ul {
  width: 48px;
}

Likely invalid or redundant comment.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
packages/settings/styles/src/components/border/BorderGroup.vue (2)

401-406: Added border style options and computed property

Border style options have been defined, and a new computed property styleValue has been added to handle the current border style selection. This change supports the new border style selection UI in the template.

Consider using a localization mechanism for the tip properties in styleOptions to support multiple languages more easily.

Also applies to: 408-411


648-705: New styles for border container

New styles have been added to support the redesigned border container layout. The use of CSS Grid for the position selector and flexbox for the inputs container provides a flexible and responsive layout.

Consider adding media queries to ensure the layout remains usable on smaller screen sizes, particularly for the gap between the position selector and inputs.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2877e00 and 999fb77.

📒 Files selected for processing (2)
  • packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (2 hunks)
  • packages/settings/styles/src/components/border/BorderGroup.vue (10 hunks)
🧰 Additional context used
🔇 Additional comments (11)
packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (4)

41-46: LGTM: Improved UI for adding new items

The new #bottom template section effectively replaces the removed meta-list-actions, simplifying the UI and directly integrating the "Add" functionality. This change aligns well with the PR objectives of refactoring styles and improving UI consistency.


175-187: LGTM: Appropriate styling for the new "Add" button

The new styles for the .add class provide appropriate styling for the newly added "Add" button, including hover effects. These changes align well with the PR objectives of refactoring styles and improving UI consistency.


Line range hint 1-187: Overall assessment: Good improvements with minor cleanup needed

The changes in this file successfully refactor the styles and improve UI consistency as per the PR objectives. The new "Add" button implementation simplifies the UI and the corresponding styles are well-implemented. However, there's still some unused code that should be removed to improve maintainability. Once the unused code is removed, this file will be in excellent shape.


54-54: ⚠️ Potential issue

Remove remaining unused code

The removal of the MetaListActions import is correct. However, as mentioned in a previous review, there are still unused elements in the script that can be removed to improve code maintainability:

  1. The actionsOptions object
  2. The actionEvents method
  3. Both actionsOptions and actionEvents from the return statement in the setup function

Please remove these unused elements to eliminate dead code.

packages/settings/styles/src/components/border/BorderGroup.vue (7)

10-16: Improved radius selection UI

The radius selection has been refactored to use a button-group component, which provides a more intuitive and consistent user interface. This change enhances the user experience by offering a clear visual representation of the available options.


18-27: Conditional rendering of single radius input

The single radius input is now conditionally rendered based on the selected radius type. This change improves the UI by showing only relevant inputs to the user.


29-82: Restructured multiple radius inputs

The multiple radius inputs have been reorganized into a more compact and visually appealing layout. Each corner now has its own section with an icon and numeric input, improving the overall user experience.


87-94: Redesigned border container layout

The border container has been significantly restructured with a new layout that separates the border position selector from the style, width, and color inputs. This new design provides a more intuitive and organized interface for managing border properties.

Also applies to: 95-177


192-192: Added ButtonGroup component

The ButtonGroup component has been imported and registered. This aligns with the template changes and provides the necessary functionality for the new UI elements.

Also applies to: 230-230


309-312: Implemented radius options and selection logic

New computed properties and methods have been added to handle radius options and selection. The selectRadius method now includes logic to reset previous radius settings when switching between single and multiple radius modes, which helps maintain consistency in the UI state.

Also applies to: 314-314, 316-335


645-646: ⚠️ Potential issue

Inconsistent CSS variable naming

The CSS variable --te-common-text-secondary is used for the border label color. This seems inconsistent with other variable names in the file that use the --ti- prefix.

Consider updating the variable name to maintain consistency:

-.border-label {
-  color: var(--te-common-text-secondary);
+  color: var(--ti-common-text-secondary);

Ensure that the CSS variable --ti-common-text-secondary is defined in your theme.

Likely invalid or redundant comment.

yy-wow
yy-wow previously approved these changes Oct 15, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (1)

Line range hint 1-187: Overall assessment: Good refactoring with minor cleanup needed

The changes in this file successfully refactor the styles and structure of the ArrayItemConfigurator component, aligning well with the PR objectives. The new "Add a Column" UI is implemented correctly, and the styling changes improve consistency and maintainability.

However, there's still some unused code related to the removed MetaListActions that should be cleaned up to further improve code quality.

Once the unused code is removed, this refactoring will be complete and ready for merging.

packages/common/component/MetaChildItem.vue (1)

90-94: Ensure consistent spacing and alignment in CSS styles

The newly added CSS rules for the .title class look good. To maintain consistency with the rest of the codebase, ensure that indentation and spacing align with the project's style guidelines.

Make sure that the opening and closing braces are properly aligned, and there are no extra spaces or tabs.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 93cf2c6 and d29a3f7.

📒 Files selected for processing (2)
  • packages/common/component/MetaChildItem.vue (5 hunks)
  • packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (3 hunks)
🧰 Additional context used
🔇 Additional comments (4)
packages/configurator/src/array-item-configurator/ArrayItemConfigurator.vue (4)

41-46: LGTM: New "Add a Column" UI implemented correctly

The new #bottom template section effectively replaces the previous meta-list-actions with a simpler UI for adding a new column. This change aligns well with the PR objectives and improves the overall user experience.


53-54: LGTM: Import statements updated correctly

The removal of the MetaListActions import and the lowercase modification for icon imports are correct and align with the template changes and past review comments.


175-187: LGTM: Styles for new "Add" button implemented correctly

The new styles for the "add" class are well-implemented and consistent with the PR objectives. The use of flexbox for layout and CSS variables for colors promotes consistency and maintainability.


54-54: ⚠️ Potential issue

Remove remaining unused code

While the MetaListActions import has been removed, there are still unused elements in the script that can be cleaned up to improve code maintainability.

Consider removing the following unused code:

  1. The actionsOptions object
  2. The actionEvents method
  3. Both actionsOptions and actionEvents from the return statement in the setup function

@chilingling chilingling merged commit 1a75cd4 into opentiny:refactor/develop Oct 15, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready_for_review This PR requires more reviews refactor-main refactor/develop branch feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants