-
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
feat: canvas 元应用改造2 (抽取CanvasLayout,面包屑导航去除元应用,api移动到engine.canvas顶层 ) #590
feat: canvas 元应用改造2 (抽取CanvasLayout,面包屑导航去除元应用,api移动到engine.canvas顶层 ) #590
Conversation
WalkthroughThe changes encompass significant updates across multiple components and modules, improving the functionality and structure of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CanvasLayout
participant DesignCanvas
participant CanvasBreadcrumb
User->>DesignCanvas: Load Design Canvas
DesignCanvas->>CanvasLayout: Initialize Layout
CanvasLayout-->>DesignCanvas: Return Layout Component
DesignCanvas->>CanvasBreadcrumb: Render Breadcrumb in Footer
CanvasBreadcrumb-->>DesignCanvas: Return Breadcrumb Component
DesignCanvas-->>User: Display Updated Canvas with Layout & Breadcrumb
Poem
Warning Review ran into problemsProblems (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked 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: 1
Outside diff range and nitpick comments (3)
packages/utils/src/utils/index.js (1)
Line range hint
129-131
: Reorder parameters to follow best practices.The default parameter should follow the last required parameter to avoid confusion and potential bugs. Here's a suggested fix:
- export function generateRandomLetters(length = 1) { + export function generateRandomLetters(length) { + length = length || 1;packages/settings/styles/src/js/useStyle.js (1)
Line range hint
81-92
: Simplify mapping logic using.flatMap()
.The current implementation uses
.map().flat()
, which can be simplified to.flatMap()
for better readability and performance.- 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/plugins/block/src/composable/useBlock.js (1)
Line range hint
275-275
: Adjust default parameter positioning in function signature.The default parameter in the function signature should follow all required parameters to avoid confusion and potential bugs during function calls.
- 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 (23)
- packages/canvas/DesignCanvas/index.js (1 hunks)
- packages/canvas/DesignCanvas/src/DesignCanvas.vue (7 hunks)
- packages/canvas/breadcrumb/src/CanvasBreadcrumb.vue (1 hunks)
- packages/canvas/container/index.js (1 hunks)
- packages/canvas/drag-drop/src/CanvasDragItem.vue (1 hunks)
- packages/canvas/index.js (1 hunks)
- packages/canvas/layout/index.js (1 hunks)
- packages/canvas/layout/meta.js (1 hunks)
- packages/canvas/layout/src/CanvasLayout.vue (1 hunks)
- packages/controller/js/constants.js (2 hunks)
- packages/controller/js/example.js (1 hunks)
- packages/controller/src/composable/index.js (1 hunks)
- packages/engine-cli/src/commands/create.js (1 hunks)
- packages/engine-cli/template/designer/registry.js (1 hunks)
- packages/engine-cli/template/designer/src/preview.js (1 hunks)
- packages/entry/src/common.js (1 hunks)
- packages/layout/index.js (1 hunks)
- packages/plugins/block/src/composable/useBlock.js (1 hunks)
- packages/plugins/datasource/index.js (1 hunks)
- packages/settings/props/src/components/inputs/CodeEditor.vue (1 hunks)
- packages/settings/props/src/components/modal/ModalContent.vue (1 hunks)
- packages/settings/styles/src/js/useStyle.js (1 hunks)
- packages/utils/src/utils/index.js (1 hunks)
Files not reviewed due to errors (1)
- packages/canvas/DesignCanvas/src/DesignCanvas.vue (no review received)
Files skipped from review due to trivial changes (13)
- packages/canvas/container/index.js
- packages/canvas/layout/index.js
- packages/canvas/layout/meta.js
- packages/canvas/layout/src/CanvasLayout.vue
- packages/controller/js/constants.js
- packages/controller/js/example.js
- packages/controller/src/composable/index.js
- packages/engine-cli/src/commands/create.js
- packages/engine-cli/template/designer/registry.js
- packages/engine-cli/template/designer/src/preview.js
- packages/entry/src/common.js
- packages/layout/index.js
- packages/plugins/datasource/index.js
Additional context used
Biome
packages/utils/src/utils/index.js
[error] 129-131: 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.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/plugins/block/src/composable/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 (10)
packages/canvas/DesignCanvas/index.js (2)
4-4
: Approved import additionThe addition of
api
from './src/api' is appropriate for the new functionalities being introduced in the export object.
7-11
: Approved modifications to export objectThe updated export structure, including
apis
andcomposable
, aligns with the PR's goal of enhancing the module's functionality and integration capabilities.packages/canvas/index.js (2)
18-18
: Approved import additionThe addition of
CanvasLayout
from './layout' is correctly implemented and aligns with the structural changes intended by the PR.
21-29
: Approved export restructuringThe restructuring of both named and default exports to include
CanvasContainer
,CanvasLayout
, andDesignCanvas
, along with modifications tocomponents
andmetas
, enhances the module's modularity and accessibility.packages/canvas/drag-drop/src/CanvasDragItem.vue (1)
19-19
: Approved API call updateUpdating the
getPluginApi
call from'engine.canvas.container'
to'engine.canvas'
correctly centralizes API access, aligning with the PR's objectives.packages/canvas/breadcrumb/src/CanvasBreadcrumb.vue (2)
4-4
: Approved template updateThe update to the template introducing conditional rendering based on
data.length
enhances user feedback and interface clarity.
24-33
: Approved script updatesThe updates to the script, including the
getPluginApi
call and the introduction of theselectFooterNode
function, enhance the component's functionality and its integration with the canvas API.packages/settings/props/src/components/modal/ModalContent.vue (1)
47-47
: Verify removal of{ emit }
from setup function signature.The removal of
{ emit }
from thesetup
function signature might affect event handling if this component emits events. Please verify through testing or checking other parts of the application where this component is used.packages/settings/props/src/components/inputs/CodeEditor.vue (1)
37-37
: Setup function implementation looks good.The setup function is well-structured and correctly uses Vue's composition API features like
reactive
,ref
, andcomputed
. Good job maintaining clean and maintainable code.packages/plugins/block/src/composable/useBlock.js (1)
23-23
: Consolidation of imports.The imports from
@opentiny/tiny-engine-entry
could potentially be consolidated into a single line to enhance readability and maintain the consistency of import statements across the project.- import { useCanvas, useTranslate, useEditorInfo, useBreadcrumb, useLayout } from '@opentiny/tiny-engine-entry' + import { useCanvas, useTranslate, useEditorInfo, useBreadcrumb, useLayout } from '@opentiny/tiny-engine-entry'Likely invalid or redundant comment.
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/canvas/breadcrumb/index.js (1 hunks)
Additional comments not posted (3)
packages/canvas/breadcrumb/index.js (3)
1-1
: Import ofHOOK_NAME
is correct.This import statement correctly pulls
HOOK_NAME
from '@opentiny/tiny-engine-entry', aligning with the project's module structure.
2-2
: Updated import forCanvasBreadcrumb
.The change in the import path to './src/CanvasBreadcrumb.vue' is consistent with the restructuring mentioned in the PR summary.
Line range hint
5-10
: Export structure review.The export default statement integrates
metaData
,apis
, andcomposable
objects effectively. The structure promotes modularity and reusability, aligning with the architectural improvements mentioned in the PR.
06d348f
to
b13aff3
Compare
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/canvas/breadcrumb/index.js (1 hunks)
- packages/canvas/index.js (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/canvas/breadcrumb/index.js
- packages/canvas/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 (1)
- packages/canvas/index.js (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/canvas/index.js
opentiny#590) * feat: canvas 元应用改造2 (抽取CanvasLayout,面包屑导航去除元应用,api移动到engine.canvas顶层 ) * fix: 解决code check问题 * fix: 修正引用路径错误 * fix: 解决解决merge过程中文件修改丢失
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
Bug Fixes
Refactor
Chores