feat：UI specification adjustment #938
Conversation
… feat/canvas-spacing
WalkthroughThis pull request introduces two new components, Changes
Suggested Labels
Suggested Reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
packages/plugins/robot/src/Main.vue (1)
Line range hint
158-171: Improve error handling in session managementThe localStorage operations should be wrapped in try-catch blocks to handle potential exceptions (e.g., QuotaExceededError). Consider extracting session management logic into a dedicated composable for better maintainability.
const setContextSession = () => { + try { localStorage.setItem( 'aiChat', sessionProcess ? JSON.stringify(sessionProcess) : JSON.stringify({ foundationModel: { manufacturer: selectedModel.value.manufacturer, model: selectedModel.value.value }, messages: [], displayMessages: [] }) ) + } catch (error) { + console.error('Failed to save chat session:', error) + // Handle the error appropriately + } }
🧹 Nitpick comments (12)
packages/plugins/page/index.js (1)
22-27: Consider using a style object instead of a stringDefining
styleas an object enhances readability and maintainability, and aligns with Vue's best practices for binding styles.Apply this diff to convert the style string into an object:
options: { pageBaseStyle: { className: 'page-base-style', - style: 'padding: 24px;background: #FFFFFF;' + style: { + padding: '24px', + background: '#FFFFFF' + } } },Using an object for styles allows easier manipulation of individual style properties and prevents potential parsing errors.
packages/builtinComponent/src/components/CanvasFlexBox.vue (2)
2-4: Add ARIA role for better accessibilityConsider adding appropriate ARIA attributes to improve accessibility, especially since this is a layout component.
- <div class="canvas-flex-box"> + <div class="canvas-flex-box" role="group" aria-label="Flex container"> <slot> </slot> </div>
44-46: Consider making flex grow/shrink configurableThe current implementation uses a fixed
flex: 1 1 0pxwhich might be too opinionated. Consider making these values configurable through props.+ flexGrow: { + type: [String, Number], + default: 1 + }, + flexShrink: { + type: [String, Number], + default: 1 + }, + flexBasis: { + type: [String, Number], + default: '0px' + },packages/plugins/materials/index.js (2)
31-38: Extract style definitions to constantsThe style definitions are hardcoded strings. Consider extracting them to named constants or a configuration file for better maintainability.
+ const BASE_STYLES = { + block: { + className: 'block-base-style', + style: 'margin: 16px;' + }, + component: { + className: 'component-base-style', + style: 'margin: 8px;' + } + } options: { ... blockBaseStyle: BASE_STYLES.block, componentBaseStyle: BASE_STYLES.component }
30-30: Document the useBaseStyle optionAdd JSDoc comments to explain the purpose and impact of the
useBaseStyleoption.+ /** + * When true, applies base styles to blocks and components + * This affects the default margin and other styling properties + * @type {boolean} + */ useBaseStyle: true,packages/builtinComponent/src/meta/CanvasSection.json (1)
63-69: Complete the snippet configurationThe snippet configuration is incomplete:
- Missing screenshot for visual reference
- Empty props object despite likely needing default styling properties
packages/builtinComponent/src/meta/CanvasFlexBox.json (3)
10-171: Consider enhancing component properties with accessibility and responsive featuresThe component properties are well-defined but could be improved with:
- Accessibility attributes (aria-label, role)
- Responsive design properties (flex-wrap, responsive padding/gap)
- Property descriptions for all fields
Consider adding these properties:
"properties": [ + { + "property": "flexWrap", + "type": "String", + "defaultValue": "nowrap", + "bindState": true, + "label": { + "text": { + "zh_CN": "换行方式" + } + }, + "widget": { + "component": "SelectConfigurator", + "props": { + "options": [ + {"label": "nowrap", "value": "nowrap"}, + {"label": "wrap", "value": "wrap"}, + {"label": "wrap-reverse", "value": "wrap-reverse"} + ] + } + } + }, + { + "property": "ariaLabel", + "type": "String", + "label": { + "text": { + "zh_CN": "无障碍标签" + } + }, + "widget": { + "component": "InputConfigurator" + } + } ]
196-203: Document nesting rules more explicitlyThe current nesting rules are too permissive with empty whitelist/blacklist arrays. Consider documenting allowed/disallowed child components to prevent invalid compositions.
"nestingRule": { - "childWhitelist": [], - "descendantBlacklist": [] + "childWhitelist": ["CanvasSection", "CanvasButton", "CanvasText"], + "descendantBlacklist": ["CanvasFlexBox"] }
205-220: Consider adding more comprehensive snippet examplesThe current snippet only includes basic properties. Consider adding more complete examples that showcase different layout patterns.
"schema": { "componentName": "CanvasFlexBox", "props": { "flexDirection": "row", "gap": "8px", - "padding": "8px" + "padding": "8px", + "justifyContent": "space-between", + "alignItems": "center" } }packages/utils/src/utils/index.js (1)
391-393: Consider optimizing the toCamelCase function.The function works correctly but could be more efficient by using a single regex operation.
-export const toCamelCase = (str) => { - return str.replace(/[-\s]+(.)?/g, (match, group1) => (group1 ? group1.toUpperCase() : '')) -} +export const toCamelCase = (str) => { + return str.replace(/[-\s](.)/g, (_, char) => char.toUpperCase()) +}packages/settings/styles/src/js/useStyle.js (1)
310-316: Consider extracting base class configuration.The base class group configuration should be moved to a constant or configuration file for better maintainability.
+const BASE_CLASS_CONFIG = { + page: () => getOptions('engine.plugins.appmanage').pageBaseStyle.className, + block: () => getOptions('engine.plugins.materials').blockBaseStyle.className, + component: () => getOptions('engine.plugins.materials').componentBaseStyle.className +} + +const getBaseClassGroup = () => [ + `.${BASE_CLASS_CONFIG.page()}`, + `.${BASE_CLASS_CONFIG.block()}`, + `.${BASE_CLASS_CONFIG.component()}` +]packages/settings/styles/src/components/typography/TypographyGroup.vue (1)
41-59: Consider expanding the font size range.While the current size range (9px to 24px) covers common use cases, consider adding larger sizes for headings (e.g., 32px, 48px).
const sizeOptions = [ // ... existing options ... + { + label: '32', + value: '32' + }, + { + label: '48', + value: '48' + } ]Also applies to: 336-373
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/builtinComponent/index.js(1 hunks)packages/builtinComponent/src/components/CanvasFlexBox.vue(1 hunks)packages/builtinComponent/src/components/CanvasSection.vue(1 hunks)packages/builtinComponent/src/meta/CanvasFlexBox.json(1 hunks)packages/builtinComponent/src/meta/CanvasRowColContainer.json(1 hunks)packages/builtinComponent/src/meta/CanvasSection.json(1 hunks)packages/builtinComponent/src/meta/index.js(1 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.js(2 hunks)packages/canvas/render/src/builtin/CanvasFlexBox.vue(1 hunks)packages/canvas/render/src/builtin/builtin.json(1 hunks)packages/canvas/render/src/render.js(2 hunks)packages/plugins/materials/index.js(1 hunks)packages/plugins/materials/src/composable/useMaterial.js(1 hunks)packages/plugins/page/index.js(1 hunks)packages/plugins/page/src/Main.vue(2 hunks)packages/plugins/page/src/PageSetting.vue(2 hunks)packages/plugins/page/src/composable/usePage.js(3 hunks)packages/plugins/robot/src/Main.vue(2 hunks)packages/settings/styles/src/components/spacing/SpacingSetting.vue(1 hunks)packages/settings/styles/src/components/typography/TypographyGroup.vue(8 hunks)packages/settings/styles/src/js/useStyle.js(5 hunks)packages/utils/src/utils/index.js(1 hunks)packages/vue-generator/src/constant/index.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/builtinComponent/src/components/CanvasSection.vue
🔇 Additional comments (16)
packages/canvas/DesignCanvas/src/api/useCanvas.js (1)
16-16: Import statement is correct
The addition of useHistory and getMetaApi from @opentiny/tiny-engine-meta-register ensures that the necessary APIs are available for use within the file.
packages/builtinComponent/index.js (1)
4-5: New component exports are correctly added
The components CanvasFlexBox and CanvasSection are properly exported, making them available for use in the application.
packages/builtinComponent/src/meta/index.js (1)
4-5: Components and snippets are correctly integrated
The new components CanvasFlexBox and CanvasSection are appropriately imported and added to the components array. Their snippets are also included in the children array under the 'layout' group.
Also applies to: 8-14, 21-21
packages/plugins/page/src/Main.vue (1)
72-72: Verify impact of getDefaultPage implementation
The switch from static to dynamic default page configuration could affect page creation behavior.
packages/settings/styles/src/components/spacing/SpacingSetting.vue (1)
48-48: LGTM! Improved spacing scale
The new spacing values follow a more consistent 4px increment pattern, which aligns better with modern design system practices and provides more fine-grained control over layout spacing.
packages/settings/styles/src/js/useStyle.js (1)
258-258: Ensure style object immutability.
Good use of object spread to maintain immutability. This prevents unintended side effects.
Also applies to: 291-291
packages/plugins/materials/src/composable/useMaterial.js (1)
61-68: Verify base style application across components.
The implementation correctly applies base styles based on component type, but we should verify the impact across different component types.
Also applies to: 73-73
✅ Verification successful
Based on the search results, I can now generate the final response:
Base style application is correctly implemented across component types
The implementation correctly handles base styles for different component types:
- Regular components use
componentBaseStyle.className - Block components use
blockBaseStyle.className - Both are conditionally applied based on
useBaseStyleoption
The base style application is consistent and properly differentiated across component types, with no issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify base style usage across components
# Check for base style class usage in component files
rg -l "className.*baseStyle" --type vue
# Check for component type definitions
ast-grep --pattern 'componentType = "Block"'
Length of output: 116
Script:
#!/bin/bash
# Let's try a different approach to verify base style usage
# Search for baseStyle usage in all files
rg "baseStyle" -A 2 -B 2
# Search for className assignments in component files
rg "className.*=" -A 2 -B 2
# Look for component type definitions with a more generic pattern
ast-grep --pattern 'componentType ='
Length of output: 27972
Script:
#!/bin/bash
# Let's search for base style related configurations and implementations
# Search for baseStyle related configurations
rg "baseStyle" -A 3 -B 3
# Search for component type definitions and their style applications
ast-grep --pattern 'componentType = $_'
# Look for style options and configurations
rg "getOptions.*Style" -A 2 -B 2
Length of output: 1340
packages/plugins/page/src/PageSetting.vue (1)
124-124: LGTM! Verify the behavior of getDefaultPage.
The switch from static DEFAULT_PAGE to dynamic getDefaultPage improves flexibility. The implementation looks correct.
Let's verify the implementation of getDefaultPage:
Also applies to: 164-164
✅ Verification successful
Based on the gathered information, I can now generate the final response:
Implementation of getDefaultPage looks correct and well-integrated
The getDefaultPage function in packages/plugins/page/src/composable/usePage.js properly extends the base DEFAULT_PAGE configuration with dynamic styles and properties. It's correctly used across multiple components:
- Main page creation in
PageSetting.vue - Temporary page creation in robot plugin
- Canvas API integration via
useCanvas.js
The switch from static to dynamic configuration maintains backward compatibility while adding flexibility through the options system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the implementation of getDefaultPage function
# Test: Search for the getDefaultPage implementation
ast-grep --pattern 'function getDefaultPage() {
$$$
}'
Length of output: 58
Script:
#!/bin/bash
# Let's try different approaches to find getDefaultPage implementation
# 1. Search for export declaration
ast-grep --pattern 'export $_ getDefaultPage'
# 2. Search for const/let/var declaration
ast-grep --pattern '$_ getDefaultPage = $_'
# 3. Search using ripgrep for any occurrence
rg "getDefaultPage" -A 5
Length of output: 4199
Script:
#!/bin/bash
# Let's check the DEFAULT_PAGE constant and its usage
rg "DEFAULT_PAGE =" -A 5
# Also check for the getOptions function that's used in getDefaultPage
ast-grep --pattern 'function getOptions($_) {
$$$
}'
# Check if there's any direct export of DEFAULT_PAGE
rg "export.*DEFAULT_PAGE"
Length of output: 998
packages/canvas/render/src/builtin/builtin.json (1)
471-471: LGTM! Appropriate default display style for Text component.
Setting display: inline-block is a good default as it provides better control over spacing and alignment while maintaining inline flow.
packages/settings/styles/src/components/typography/TypographyGroup.vue (3)
190-205: Good addition of Chinese font support!
The addition of Chinese fonts enhances internationalization support. The font stack is well-structured with appropriate fallbacks.
585-589: LGTM! Clean layout improvements.
The grid layout adjustments and font size display styling enhance the UI's visual organization and alignment.
Also applies to: 686-692
524-528: Verify font size unit handling.
The font size handler appends 'px' unit. Ensure this works correctly with other potential units (em, rem) if supported.
packages/plugins/robot/src/Main.vue (2)
Line range hint 392-668: LGTM: Well-structured UI implementation
The chat window UI implementation demonstrates good practices:
- Proper use of CSS variables for theming
- Well-organized layout structure
- Responsive design considerations
149-149:
Breaking change: DEFAULT_PAGE constant replaced with getDefaultPage function
The change from using a constant DEFAULT_PAGE to a function getDefaultPage() is a breaking change that affects page creation. While this change improves encapsulation, we should ensure all other components using DEFAULT_PAGE are updated accordingly.
Also applies to: 177-177
packages/canvas/render/src/render.js (1)
20-26: LGTM: Clean integration of new components
The addition of CanvasFlexBox and CanvasSection components is well-integrated:
- Proper import organization
- Consistent Mapper registration
- Maintains existing patterns
Also applies to: 75-76
packages/vue-generator/src/constant/index.js (1)
2198-2211: LGTM: Consistent component registration
The new components are properly registered in BUILTIN_COMPONENTS_MAP with consistent:
- Version numbering
- Package references
- Destructuring configuration
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/builtinComponent/src/components/CanvasFlexBox.vue (1)
11-32: 🛠️ Refactor suggestionAdd prop validation and improve type safety
The flex-related props would benefit from validation and TypeScript types for better type safety and developer experience.
+type FlexDirection = 'row' | 'row-reverse' | 'column' | 'column-reverse' +type AlignItems = 'flex-start' | 'flex-end' | 'center' | 'baseline' | 'stretch' +type JustifyContent = 'flex-start' | 'flex-end' | 'center' | 'space-between' | 'space-around' | 'space-evenly' const props = defineProps({ flexDirection: { - type: String, + type: String as PropType<FlexDirection>, default: 'row', + validator: (value: string) => ['row', 'row-reverse', 'column', 'column-reverse'].includes(value) }, gap: { - type: [String, Number], + type: [String, Number] as PropType<string | number>, default: '8px', + validator: (value: string | number) => { + if (typeof value === 'number') return value >= 0 + return /^(\d+(\.\d+)?)(px|rem|em|%)$/.test(value) + } }, padding: { - type: [String, Number], + type: [String, Number] as PropType<string | number>, default: '8px', + validator: (value: string | number) => { + if (typeof value === 'number') return value >= 0 + return /^(\d+(\.\d+)?)(px|rem|em|%)$/.test(value) + } }, alignItems: { - type: String, + type: String as PropType<AlignItems>, default: 'center', + validator: (value: string) => ['flex-start', 'flex-end', 'center', 'baseline', 'stretch'].includes(value) }, justifyContent: { - type: String, + type: String as PropType<JustifyContent>, default: 'flex-start', + validator: (value: string) => ['flex-start', 'flex-end', 'center', 'space-between', 'space-around', 'space-evenly'].includes(value) } })
🧹 Nitpick comments (3)
packages/builtinComponent/src/components/CanvasFlexBox.vue (3)
1-5: Remove unnecessary space in slot elementThe space within the slot element is not needed and can be removed for cleaner code.
- <slot> </slot> + <slot></slot>
9-9: Use more specific import pathConsider using a more specific import path for better maintainability.
-import { getStyleValue } from './helper' +import { getStyleValue } from './utils/style-helper'
43-54: Document deep selector usage and consider flex property implicationsA few suggestions for the styles:
- Document why the deep selector is needed for
.canvas-container- Consider making flex-grow configurable via props as it might cause unexpected behavior in some layouts
- Use individual flex properties for better clarity
.canvas-flex-box { display: flex; - flex: 1 1 0px; + // These can be made configurable via props if needed + flex-grow: 1; + flex-shrink: 1; + flex-basis: 0px; flex-direction: v-bind('styles.flexDirection'); gap: v-bind('styles.gap'); padding: v-bind('styles.padding'); align-items: v-bind('styles.alignItems'); justify-content: v-bind('styles.justifyContent'); + + // Deep selector needed to ensure consistent width for nested canvas containers :deep(.canvas-container) { width: 100%; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/builtinComponent/src/components/CanvasFlexBox.vue(1 hunks)
🔇 Additional comments (1)
packages/builtinComponent/src/components/CanvasFlexBox.vue (1)
1-55: Verify component registration and metadata
Please ensure that:
- The component is properly registered in the main export file
- The metadata file exists with correct configuration
✅ Verification successful
Let me verify the content of these files to ensure complete integration.
Component integration is properly configured
The verification confirms that the CanvasFlexBox component is correctly integrated:
- Properly exported in
packages/builtinComponent/index.js - Complete metadata file exists with correct props configuration matching the component
- Correctly mapped in the component registry with proper package reference
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check component registration and metadata
echo "Checking component registration..."
rg -l "export.*CanvasFlexBox" packages/builtinComponent/
echo "Checking metadata file..."
fd "CanvasFlexBox.json" packages/builtinComponent/src/meta/
echo "Checking component map..."
rg "CanvasFlexBox" packages/vue-generator/src/constant/
Length of output: 623
Script:
#!/bin/bash
# Check the content of registration and metadata files
echo "Component registration in index.js:"
cat packages/builtinComponent/index.js | grep -A 5 -B 5 "CanvasFlexBox"
echo -e "\nMetadata content:"
cat packages/builtinComponent/src/meta/CanvasFlexBox.json
echo -e "\nComponent map entry:"
cat packages/vue-generator/src/constant/index.js | grep -A 10 -B 2 "CanvasFlexBox"
Length of output: 6879
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/plugins/page/src/composable/usePage.js (1)
61-69: Consider improving string template formattingWhile the function logic is correct, the string concatenation could be more readable using template literals with proper indentation.
Consider this improvement:
- const baseStyle = `.${pageOptions.pageBaseStyle.className}{\r\n ${pageOptions.pageBaseStyle.style}\r\n}\r\n` + const baseStyle = ` + .${pageOptions.pageBaseStyle.className} { + ${pageOptions.pageBaseStyle.style} + } + ` - return `${baseStyle}.${materialsOptions.blockBaseStyle.className}{\r\n ${materialsOptions.blockBaseStyle.style}\r\n}\r\n.${materialsOptions.componentBaseStyle.className}{\r\n ${materialsOptions.componentBaseStyle.style}\r\n}\r\n` + return ` + ${baseStyle} + .${materialsOptions.blockBaseStyle.className} { + ${materialsOptions.blockBaseStyle.style} + } + .${materialsOptions.componentBaseStyle.className} { + ${materialsOptions.componentBaseStyle.style} + } + `packages/utils/src/utils/index.js (1)
391-393: Consider enhancing separator support in toCamelCase.The function handles hyphens and spaces but could be more robust by supporting additional common separators like underscores.
export const toCamelCase = (str) => { - return str.replace(/[-\s]+(.)?/g, (match, group1) => (group1 ? group1.toUpperCase() : '')) + return str.replace(/[-_\s]+(.)?/g, (match, group1) => (group1 ? group1.toUpperCase() : '')) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/canvas/DesignCanvas/src/api/useCanvas.js(2 hunks)packages/plugins/page/src/Main.vue(2 hunks)packages/plugins/page/src/composable/usePage.js(3 hunks)packages/utils/src/utils/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/page/src/Main.vue
🔇 Additional comments (4)
packages/canvas/DesignCanvas/src/api/useCanvas.js (2)
16-16: LGTM: Clean import addition
The addition of getMetaApi from @opentiny/tiny-engine-meta-register aligns with the architectural changes for metadata management.
83-93: LGTM: Robust implementation with proper error handling
The implementation includes proper null checks and default values as previously suggested, preventing potential runtime errors from undefined properties.
packages/plugins/page/src/composable/usePage.js (2)
15-15: LGTM: Clean import addition
The addition of getOptions from @opentiny/tiny-engine-meta-register aligns with the architectural changes for options management.
166-166: LGTM: Clean export addition
The addition of getDefaultPage to the exports is consistent with the architectural changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/plugins/page/src/composable/usePage.js (1)
71-90: Enhance error handling with specific messagesGood implementation that follows previous review suggestions for immutability and basic error handling. Consider enhancing the error handling further:
const getDefaultPage = () => { const materialsOptions = getOptions('engine.plugins.materials') const pageOptions = getOptions('engine.plugins.appmanage') - if (!materialsOptions || !pageOptions || !pageOptions.pageBaseStyle) { + if (!materialsOptions) { + console.warn('Missing materials options') + return { ...DEFAULT_PAGE } + } + + if (!pageOptions?.pageBaseStyle) { + console.warn('Missing or invalid page options') return { ...DEFAULT_PAGE } } return { ...DEFAULT_PAGE, page_content: { ...DEFAULT_PAGE.page_content, props: { ...DEFAULT_PAGE.page_content.props, className: pageOptions.pageBaseStyle.className }, css: generateCssString(pageOptions, materialsOptions) } } }packages/utils/src/utils/index.js (1)
384-448: Consider consolidating similar utility functions.The file contains multiple functions with overlapping functionality:
toCamelCasevscamelizeconvertCamelToKebabvshyphenateConsider consolidating these functions to reduce code duplication and maintain a single source of truth for each transformation type.
Suggested approach:
- Use the existing cached versions (
camelizeandhyphenate) as the primary functions- If the new implementations offer improvements, merge those improvements into the existing functions
- Update all usages to reference the consolidated functions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/page/src/composable/usePage.js(3 hunks)packages/utils/src/utils/index.js(1 hunks)
🔇 Additional comments (4)
packages/plugins/page/src/composable/usePage.js (2)
15-15: LGTM!
The import statement is correctly placed and follows project conventions.
170-170: LGTM!
The export is correctly placed and properly includes the new function.
packages/utils/src/utils/index.js (2)
414-431: LGTM! Implementation follows best practices.
The function includes proper input validation and handles complex CSS values correctly.
439-448: LGTM! Implementation follows best practices.
The function includes proper input validation and handles null/undefined values correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/builtinComponent/src/meta/CanvasFlexBox.json (2)
173-188: Consider adding more interaction events.For a container component that might be used in a drag-and-drop interface, consider adding the following events:
onDragOver/onDropfor drag-and-drop functionalityonMouseEnter/onMouseLeavefor hover effectsExample addition:
"events": { "onClick": { "label": { "zh_CN": "点击事件" }, "description": { "zh_CN": "点击时触发的回调函数" }, "type": "event", "functionInfo": { "params": [], "returns": {} }, "defaultValue": "" }, + "onDrop": { + "label": { + "zh_CN": "放置事件" + }, + "description": { + "zh_CN": "拖拽元素放置时触发的回调函数" + }, + "type": "event", + "functionInfo": { + "params": [{"name": "event", "type": "DragEvent"}], + "returns": {} + }, + "defaultValue": "" + } }
205-220: Add a screenshot for better visual reference.The
screenshotfield is empty. Adding a screenshot would help users understand the component's appearance and behavior in the UI builder.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/builtinComponent/src/meta/CanvasFlexBox.json(1 hunks)packages/builtinComponent/src/meta/CanvasSection.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/builtinComponent/src/meta/CanvasSection.json
🔇 Additional comments (2)
packages/builtinComponent/src/meta/CanvasFlexBox.json (2)
1-7: LGTM! Component basic information is well-defined.
The component naming and metadata are appropriate and consistent with the component's purpose.
196-203: Define nesting rules for better component compatibility.
The current configuration has empty nesting rules (childWhitelist and descendantBlacklist). Consider defining these rules to prevent incompatible component combinations and potential layout issues.
Let's check what components are typically used with flex containers:
packages/settings/styles/src/components/typography/TypographyGroup.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/settings/styles/src/components/typography/TypographyGroup.vue (3)
50-59: Consider internationalizing the "PX" labelThe "PX" label should be internationalized to support multiple languages.
- PX + {{ t('unit.pixels') }}
190-205: Consider grouping fonts by language/scriptThe font options could be better organized by grouping them into categories (Chinese, Latin, etc.) for better maintainability and user experience.
const fontFamilyOptions = [ + // Chinese Fonts { label: '微软雅黑', value: '"Microsoft YaHei", "微软雅黑", sans-serif' }, // ... other Chinese fonts ... + // Latin Fonts { label: 'Arial', value: 'Arial, "Helvetica Neue", Helvetica' }, // ... other Latin fonts ... ]
549-552: Consider using CSS variables for consistent spacing and colorsMove hardcoded values to CSS variables for better maintainability and consistency across the application.
&.font-split { - gap: 4px 8px; + gap: var(--te-common-space-4) var(--te-common-space-8); grid-template-columns: 56% auto; } .font-size { display: flex; - font-size: 12px; + font-size: var(--te-common-font-size-small); color: var(--te-common-text-weaken); align-items: center; - gap: 4px; + gap: var(--te-common-space-4); }Also applies to: 650-656
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/settings/styles/src/components/typography/TypographyGroup.vue(8 hunks)
🔇 Additional comments (1)
packages/settings/styles/src/components/typography/TypographyGroup.vue (1)
336-337: LGTM! Efficient implementation of size options
The implementation follows the suggested optimization using map to convert sizes to options.
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
What is the current behavior?
Issue Number: N/A
What is the new behavior?
1、创建页面时页面自带页面、区块、组件的基础样式
2、拖入区块、组件默认添加基础样式类名
3、清空画布保留基础样式
4、在样式面板调整组件/区块新增一个类名，不会在基础样式类名上修改
5、添加两个容器组件（弹性布局组件、默认全宽+居中的组件）
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
CanvasFlexBoxandCanvasSection, enhancing layout options.Bug Fixes
Enhancements
Utility Functions