-
Notifications
You must be signed in to change notification settings - Fork 347
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(material): 物料管理插件元应用拆分 #545
Conversation
WalkthroughThis update significantly enhances the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- designer-demo/registry.js (1 hunks)
- packages/design-core/src/App.vue (1 hunks)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/block/index.js (1 hunks)
- packages/plugins/materials/src/meta/block/meta.js (1 hunks)
- packages/plugins/materials/src/meta/block/src/Main.vue (3 hunks)
- packages/plugins/materials/src/meta/block/src/js/usePanel.js (1 hunks)
- packages/plugins/materials/src/meta/component/index.js (1 hunks)
- packages/plugins/materials/src/meta/component/meta.js (1 hunks)
- packages/plugins/materials/src/meta/contentLayout/index.js (1 hunks)
- packages/plugins/materials/src/meta/contentLayout/meta.js (1 hunks)
- packages/plugins/materials/src/meta/contentLayout/src/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/header/index.js (1 hunks)
- packages/plugins/materials/src/meta/header/meta.js (1 hunks)
- packages/plugins/materials/src/meta/header/src/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/layout/index.js (1 hunks)
- packages/plugins/materials/src/meta/layout/meta.js (1 hunks)
- packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files skipped from review due to trivial changes (6)
- packages/plugins/materials/src/meta/block/index.js
- packages/plugins/materials/src/meta/block/meta.js
- packages/plugins/materials/src/meta/block/src/js/usePanel.js
- packages/plugins/materials/src/meta/contentLayout/index.js
- packages/plugins/materials/src/meta/header/index.js
- packages/plugins/materials/src/meta/header/meta.js
Additional comments not posted (17)
packages/plugins/materials/src/meta/layout/meta.js (1)
1-4
: Metadata definition for layout component looks correct and aligns with the plugin's structure.packages/plugins/materials/src/meta/contentLayout/meta.js (1)
1-4
: Metadata definition for content layout component looks correct and aligns with the plugin's structure.packages/plugins/materials/src/meta/component/meta.js (1)
1-4
: Metadata definition for materials component looks correct and aligns with the plugin's structure.packages/plugins/materials/src/meta/layout/index.js (1)
1-7
: Proper integration of component and metadata for layout. Good use of spread operator for modularity.packages/plugins/materials/src/meta/component/index.js (1)
1-10
: Proper integration of component, metadata, and options for materials. Good use of spread operator for modularity.packages/plugins/materials/src/Main.vue (1)
2-2
: Dynamic component rendering based on layout and metadata is implemented correctly. Proper use ofgetMergeRegistry
for fetching and merging registry information.Also applies to: 6-14
packages/plugins/materials/src/meta/header/src/Main.vue (2)
1-8
: Template implementation looks clean and follows Vue best practices.
11-33
: Script setup and component registration are correctly implemented.packages/plugins/materials/index.js (1)
13-19
: Import statements are correctly organized and enhance modularity.packages/plugins/materials/src/meta/layout/src/Main.vue (2)
1-9
: Template uses dynamic components effectively, allowing for flexible UI customization.
12-56
: Script correctly utilizes Vue's composition API and reactive state management.packages/plugins/materials/src/meta/contentLayout/src/Main.vue (2)
1-9
: Template effectively uses conditional rendering and dynamic components.
11-46
: Script is well-organized and makes effective use of Vue's reactivity and composition API.designer-demo/registry.js (1)
61-75
: Metadata entries for the materials plugin are correctly added and enhance configurability.packages/design-core/src/App.vue (1)
46-46
: Integration offetchGroups
from the materials plugin is correctly implemented.packages/plugins/materials/src/meta/block/src/Main.vue (2)
131-138
: Review the logic in the new watch function forprops.activeTabName
to ensure it aligns with component requirements.Verification successful
The extracted code provides the necessary context to verify the logic of the watch function for
props.activeTabName
. The function watches for changes inprops.activeTabName
and, if the new value does not matchmetaData.id
, it setssetBlockPanelVisible
andsetBlockVersionPanelVisible
tofalse
.This logic appears to be correct and aligns with the typical use of a watch function in Vue.js to react to changes in a prop and update the component's state accordingly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic of the watch function for `props.activeTabName`. # Test: Search for the logic handling in the watch function. Expect: Correct handling based on `metaData.id`. rg --type vue $'props.activeTabName' packages/plugins/materials/src/meta/block/src/Main.vueLength of output: 118
Script:
#!/bin/bash # Description: Verify the logic of the watch function for `props.activeTabName`. # Test: Search for the logic handling in the watch function. Expect: Correct handling based on `metaData.id`. rg 'props.activeTabName' packages/plugins/materials/src/meta/block/src/Main.vueLength of output: 112
Script:
#!/bin/bash # Description: Extract the watch function for `props.activeTabName` and its surrounding code for detailed analysis. # Extract the watch function and its surrounding code. rg -A 10 -B 10 'props.activeTabName' packages/plugins/materials/src/meta/block/src/Main.vueLength of output: 498
39-39
: Verify the necessity and usage of the new proprightPanelRef
.Verification successful
The prop
rightPanelRef
is used within a<teleport>
directive in the component. This confirms its necessity and usage.
- Location of usage:
- Line containing
<teleport to=".material-right-panel" v-if="rightPanelRef">
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the prop `rightPanelRef` in the component. # Test: Search for the prop usage in the component. Expect: At least one occurrence of the prop. rg --type vue $'rightPanelRef' packages/plugins/materials/src/meta/block/src/Main.vueLength of output: 112
Script:
#!/bin/bash # Description: Verify the usage of the prop `rightPanelRef` in the component. # Test: Search for the prop usage in the component. Expect: At least one occurrence of the prop. rg --type-add 'vue:*.vue' rightPanelRef packages/plugins/materials/src/meta/block/src/Main.vueLength of output: 183
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/plugins/materials/index.js
- packages/plugins/materials/src/meta/layout/src/Main.vue
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/materials/src/meta/contentLayout/index.js (1 hunks)
- packages/plugins/materials/src/meta/contentLayout/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/plugins/materials/src/meta/contentLayout/index.js
- packages/plugins/materials/src/meta/contentLayout/src/Main.vue
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/Main.vue (1 hunks)
- packages/plugins/materials/src/components/header/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/block/index.js (1 hunks)
- packages/plugins/materials/src/meta/component/index.js (1 hunks)
- packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files not summarized due to errors (1)
- packages/plugins/materials/src/meta/component/index.js: Error: Server error. Please try again later.
Files not reviewed due to errors (3)
- packages/plugins/materials/src/meta/component/index.js (Error: Server error. Please try again later.)
- packages/plugins/materials/src/Main.vue (Error: Server error. Please try again later.)
- packages/plugins/materials/src/components/header/Main.vue (Error: Server error. Please try again later.)
Files skipped from review as they are similar to previous changes (3)
- packages/plugins/materials/index.js
- packages/plugins/materials/src/meta/block/index.js
- packages/plugins/materials/src/meta/layout/src/Main.vue
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/design-core/src/layout/App.vue (1 hunks)
Additional comments not posted (1)
packages/design-core/src/layout/App.vue (1)
46-46
: Ensure that the updated import pathmaterials.apis
is consistently used across the project.Verification successful
The updated import path
materials.apis
is correctly used inpackages/design-core/src/layout/App.vue
, and there are no occurrences of the old pathmaterials.api
in the.vue
files.
- : The import path
materials.apis
is consistently used across the project. No occurrences of the old pathmaterials.api
were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the updated import path `materials.apis` is used consistently across the project. # Test: Search for the old and new import paths. Expect: No occurrences of the old path. rg --type vue $'materials.api' rg --type vue $'materials.apis'Length of output: 116
Script:
#!/bin/bash # Description: Verify that the updated import path `materials.apis` is used consistently across the project. # Test: Search for the old and new import paths. Expect: No occurrences of the old path. rg 'materials.api' --glob '*.vue' rg 'materials.apis' --glob '*.vue'Length of output: 226
There was a problem hiding this 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/js/useStyle.js (1)
Line range hint
81-92
: Simplify the.map().flat()
call to.flatMap()
for better performance and readability.- res = expressRes - .map((item) => { - if (typeof item === 'string') { - return item - } - if (typeof item === 'object') { - return Object.keys(item) - } - return null - }) - .flat() - .filter(Boolean) + res = expressRes.flatMap((item) => { + if (typeof item === 'string') { + return item + } + if (typeof item === 'object') { + return Object.keys(item) + } + return [] + }).filter(Boolean)packages/controller/src/hooks/useBlock.js (1)
Line range hint
275-275
: The default parameter should follow the last required parameter or be turned into a required parameter to avoid confusion and potential bugs.- const initBlock = async (block = {}, _langs = {}, isEdit) => { + const initBlock = async (block, _langs = {}, isEdit) => {Consider revising the function signature to ensure the default parameter follows the best practices.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- packages/controller/src/hooks/useBlock.js (1 hunks)
- packages/design-core/index.js (1 hunks)
- packages/entry/src/common.js (1 hunks)
- packages/layout/src/Main.vue (1 hunks)
- packages/settings/styles/src/js/useStyle.js (1 hunks)
Files skipped from review due to trivial changes (2)
- packages/design-core/index.js
- packages/entry/src/common.js
Additional context used
Biome
packages/settings/styles/src/js/useStyle.js
[error] 81-92: The call chain .map().flat() can be replaced with a single .flatMap() call. (lint/complexity/useFlatMap)
Safe fix: Replace the chain with .flatMap().
packages/controller/src/hooks/useBlock.js
[error] 275-275: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 275-275: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Additional comments not posted (3)
packages/layout/src/Main.vue (1)
53-53
: UpdatedfetchGroups
import path tomaterials.apis
aligns with the PR's objective to enhance plugin architecture.packages/settings/styles/src/js/useStyle.js (1)
137-143
: Enhanced thewatch
function to monitor multiple properties effectively. This change will help in maintaining the style state accurately across components.packages/controller/src/hooks/useBlock.js (1)
23-23
: Optimized imports by consolidating them into a single line, which improves readability and reduces clutter.
There was a problem hiding this 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/js/useStyle.js (1)
Line range hint
81-92
: Consider simplifying the.map().flat()
calls to.flatMap()
for better performance and readability in thegetPropsFromExpression
function.- res = expressRes.map((item) => { - if (typeof item === 'string') { - return item - } - if (typeof item === 'object') { - return Object.keys(item) - } - return null - }).flat().filter(Boolean) + res = expressRes.flatMap((item) => { + if (typeof item === 'string') { + return item + } + if (typeof item === 'object') { + return Object.keys(item) + } + return [] + }).filter(Boolean)packages/controller/src/hooks/useBlock.js (1)
Line range hint
275-275
: Consider reordering the default parameter in theinitBlock
function to follow the last required parameter for better code style and clarity.- const initBlock = async (block = {}, _langs = {}, isEdit) => { + const initBlock = async (block = {}, isEdit, _langs = {}) => {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- packages/controller/src/hooks/useBlock.js (1 hunks)
- packages/design-core/index.js (1 hunks)
- packages/entry/src/common.js (1 hunks)
- packages/layout/src/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/component/src/CanvasDragItem.vue (1 hunks)
- packages/plugins/materials/src/meta/component/src/Main.vue (1 hunks)
- packages/plugins/materials/src/meta/layout/index.js (1 hunks)
- packages/settings/styles/src/js/useStyle.js (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- packages/design-core/index.js
- packages/entry/src/common.js
- packages/layout/src/Main.vue
- packages/plugins/materials/src/meta/layout/index.js
Additional context used
Biome
packages/settings/styles/src/js/useStyle.js
[error] 81-92: The call chain .map().flat() can be replaced with a single .flatMap() call. (lint/complexity/useFlatMap)
Safe fix: Replace the chain with .flatMap().
packages/controller/src/hooks/useBlock.js
[error] 275-275: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
[error] 275-275: This default parameter should follow the last required parameter or should be a required parameter. (lint/style/useDefaultParameterLast)
The last required parameter is here:
A default parameter that precedes a required parameter cannot be omitted at call site.
Unsafe fix: Turn the parameter into a required parameter.
Additional comments not posted (2)
packages/plugins/materials/src/meta/component/src/CanvasDragItem.vue (1)
1-51
: The implementation of the draggable component inCanvasDragItem.vue
looks solid. The drag and click functionalities are correctly implemented, and the component is styled appropriately for its purpose.packages/plugins/materials/src/meta/component/src/Main.vue (1)
Line range hint
1-76
: The implementation of the component management interface inMain.vue
is well-structured. The use of search and collapsible functionalities enhances the user experience, and the integration withCanvasDragItem
for drag-and-drop capabilities is effectively handled.
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/plugins/materials/index.js
Additional comments not posted (4)
packages/plugins/materials/src/Main.vue (4)
13-14
: Confirm thatregistryData
andlayout
are used effectively and check for potential reactivity issues in the template.Verification successful
The previous script failed because the file type
vue
was not recognized. To proceed, we should search for the usage ofregistryData
andlayout
within the Vue file without specifying the file type.
registryData
andlayout
are effectively used in the template within the<component>
tag.
registryData
is passed as a prop:<component :is="layout.entry" :registryData="registryData"></component>
.layout
is used to dynamically set the component:<component :is="layout.entry" :registryData="registryData"></component>
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the effective use and reactivity of `registryData` and `layout`. # Test: Search for the usage of `registryData` and `layout` in the template. rg --type vue $'registryData' packages/plugins/materials/src/Main.vue rg --type vue $'layout' packages/plugins/materials/src/Main.vueLength of output: 187
Script:
#!/bin/bash # Description: Verify the effective use and reactivity of `registryData` and `layout`. # Test: Search for the usage of `registryData` and `layout` in the template. rg 'registryData' packages/plugins/materials/src/Main.vue rg 'layout' packages/plugins/materials/src/Main.vueLength of output: 353
6-7
: Validate thatgetMergeRegistry
andmeta
are used appropriately and check if all imported entities are utilized.Verification successful
The imported modules
getMergeRegistry
andmeta
are used within thepackages/plugins/materials/src/Main.vue
file, specifically in the lineconst registry = getMergeRegistry(meta.type, meta.id)
. This confirms that both imports are utilized appropriately.
getMergeRegistry
is called with parameters derived frommeta
.meta
is used to provide thetype
andid
arguments for thegetMergeRegistry
function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of imported modules. # Test: Search for the usage of `getMergeRegistry` and `meta`. rg --type vue $'getMergeRegistry' packages/plugins/materials/src/Main.vue rg --type vue $'meta' packages/plugins/materials/src/Main.vueLength of output: 189
Script:
#!/bin/bash # Description: Verify the usage of imported modules without specifying file type. # Test: Search for the usage of `getMergeRegistry` and `meta`. rg 'getMergeRegistry' packages/plugins/materials/src/Main.vue rg 'meta' packages/plugins/materials/src/Main.vueLength of output: 317
9-11
: Check the robustness of destructuring insetup
. Ensurelayout
andrestRegistry
always receive expected values.Verification successful
The recent script execution confirms that
layout
andrestRegistry
are properly handled in thesetup
function and their usage is consistent within theMain.vue
file.
- The
layout
property is used in a component binding.- The
restRegistry
is assigned toregistryData
.No issues were found with the destructuring and usage of these properties.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of destructured values in the setup function. # Test: Search for the handling and default values of `layout` and `restRegistry`. rg --type vue $'layout' packages/plugins/materials/src/Main.vue rg --type vue $'restRegistry' packages/plugins/materials/src/Main.vueLength of output: 187
Script:
#!/bin/bash # Description: Verify the handling of destructured values in the setup function. # Test: Search for the handling and default values of `layout` and `restRegistry`. rg 'layout' packages/plugins/materials/src/Main.vue rg 'restRegistry' packages/plugins/materials/src/Main.vueLength of output: 328
2-2
: Ensure thelayout.entry
andregistryData
are always provided to avoid runtime errors.
…ngine into refactor/develop
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/plugins/materials/index.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/plugins/materials/index.js
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/plugins/materials/index.js
- packages/plugins/materials/src/meta/layout/src/Main.vue
There was a problem hiding this 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/plugins/materials/index.js (1 hunks)
- packages/plugins/materials/src/meta/layout/src/Main.vue (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/plugins/materials/index.js
- packages/plugins/materials/src/meta/layout/src/Main.vue
* feat(toolbars): 工具栏增加meta.js * feat(toolbars): 工具栏增加vite-plugin-generate-comments * feat(toolbars): 工具栏增加@opentiny/tiny-engine-entry * refactor(material): 物料管理插件元应用拆分 * refactor(materials): 物料管理插件review * refactor(materials): 物料管理插件review * refactor(materials): 物料管理插件review * refactor(materials): 物料管理插件review * component->entry * refactor(material): review 意见修改 * refactor(materials): review意见修改
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?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
MaterialLayout
,MaterialBlock
,MaterialComponent
, andMaterialHeader
.Improvements
Bug Fixes
BreadcrumbService
declaration.Style