-
Notifications
You must be signed in to change notification settings - Fork 354
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:preview multi page structured data transfer and nested display #978
Conversation
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces enhancements to page generation and preview functionality across multiple packages. The changes focus on improving the dynamic generation of page structures, particularly in handling page ancestors and family relationships. Key modifications include adding a new Changes
Sequence DiagramsequenceDiagram
participant Main as Main.vue
participant UsePage as usePage
participant Preview as Preview.vue
participant Generator as Vue Generator
Main->>UsePage: getFamily(pageId)
UsePage-->>Main: Return page ancestors
Main->>Preview: Pass ancestors to preview
Preview->>Preview: Generate family pages
Preview->>Generator: Generate components with family context
Possibly related PRs
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 review |
✅ Actions performedReview triggered.
|
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
🧹 Nitpick comments (5)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (1)
197-201
: Conditionally replacingRouterView
is clever; handle multi-view scenarios.Replacing
'RouterView'
withnextPage
helps in dynamic page routing, but watch for edge-cases like multipleRouterView
instances in a single layout. Consider how the logic should behave if multiple views exist or ifcomponentName
might need further checks.packages/toolbars/preview/src/Main.vue (1)
33-33
: DestructuregetFamily
for clarity.Nice extraction of
getFamily
fromusePage
. Be mindful of future expansions in the composable, which may group related methods together.packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (1)
200-205
: ForwardnextPage
into template generation hooks.Passing
nextPage
togenTemplateByHook
can be powerful for multi-page logic. Confirm that any plugin hooks expecting fewer parameters remain backward-compatible.packages/plugins/page/src/composable/usePage.js (2)
214-228
: StandalonegetAncestorsRecursively
function is clearer.Pulling the recursive logic into its own function neatly separates concerns. This improves readability, though consider whether tail-recursive or iterative approaches might be beneficial if dealing with very deep page trees.
250-259
:getFamily
filters pages only, reversing for a top-down order.That’s a straightforward approach. It might be worth adding a default fallback if
id
is invalid to prevent possible errors or empty returns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/design-core/src/preview/src/preview/Preview.vue
(3 hunks)packages/plugins/page/src/composable/usePage.js
(3 hunks)packages/toolbars/preview/src/Main.vue
(3 hunks)packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js
(3 hunks)packages/vue-generator/src/generator/vue/sfc/generateScript.js
(3 hunks)packages/vue-generator/src/generator/vue/sfc/generateTemplate.js
(4 hunks)
🔇 Additional comments (20)
packages/vue-generator/src/generator/vue/sfc/generateTemplate.js (3)
175-175
: Introduce nextPage
parameter carefully.
Adding nextPage
as a parameter to recursiveGenTemplateByHook
broadens its functionality and looks consistent with other changes. Ensure all call sites pass the correct type, and handle the case where nextPage
might be null
or undefined
.
214-214
: Propagate nextPage
to downstream hooks.
Ensuring the nextPage
argument is passed to each hookItem
preserves the intended logic. Confirm any custom hooks can handle (or ignore) this parameter correctly.
236-242
: New usage pattern with nextPage
looks consistent.
Both genTemplateByHook
and the internal call to recursiveGenTemplateByHook
properly integrate nextPage
. The usage is coherent with the rest of the code changes.
packages/toolbars/preview/src/Main.vue (2)
15-15
: Import usePage
usage is appropriate.
Importing usePage
aligns well with the newly introduced getFamily
function. No issues noted.
83-83
: Validate the result of getFamily
.
Assigning params.ancestors = await getFamily(params.id)
is a good approach. Consider safeguarding if getFamily
returns an empty array or any unexpected structure, especially for invalid page IDs.
packages/vue-generator/src/generator/vue/sfc/genSetupSFC.js (3)
70-70
: Add nextPage
parameter to generateSFCFile
.
This additional parameter broadens the function’s scope. Verify that calling code consistently provides nextPage
where needed and handles the case if it’s absent or null.
208-208
: Script generation also respects new param.
genScriptByHook
alignment with nextPage
suggests a consistent approach across template and script generation. This looks good.
216-216
: genSFCWithDefaultPlugin
integrates nextPage
thoroughly.
The expansion of function signatures and final call to generateSFCFile
with nextPage
ensures the entire SFC pipeline can handle pagination or additional next-page context seamlessly.
Also applies to: 273-273
packages/design-core/src/preview/src/preview/Preview.vue (6)
46-46
: Synchronized export of getAllNestedBlocksSchema
and generatePageCode
.
Extracting these methods from getMetaApi
here is consistent with usage. Confirm they remain in sync with the importer’s version.
88-88
: New helper function getFamilyPages
.
Introducing getFamilyPages
to gather an array of pages based on ancestors is a clean approach. This new layer systematically organizes the page data.
89-89
: Use const
for familyPages
instead of let
.
As previously noted, the reference to familyPages
is never reassigned, so changing it to const
enhances clarity and enforces immutability.
94-94
: Replace '0'
with pageSettingState.ROOT_ID
if feasible.
As previously mentioned, referencing '0'
directly may be fragile. Using pageSettingState.ROOT_ID
is more consistent and self-documenting.
95-108
: Combine repeated logic to reduce duplication.
These two blocks differ only in a couple of fields (panelName: 'Main.vue'
and index
). You might factor out the shared logic to a helper, as previously suggested.
Also applies to: 110-123
144-144
: Insert getFamilyPages
result seamlessly.
Appending getFamilyPages(appData)
to pageCode
before blocks is well-integrated. The final array merges hierarchy-based pages with block-based pages for a consolidated preview experience.
packages/plugins/page/src/composable/usePage.js (1)
276-276
: Expose getFamily
in default export.
Exporting getFamily
for broader usage is in line with the new logic in Main.vue
and Preview.vue
. This maintains consistency across the codebase.
packages/vue-generator/src/generator/vue/sfc/generateScript.js (5)
8-8
: Ensure consistency in calling the modified function.
You've added config
and nextPage
to the defaultGenImportHook
parameters. Verify that all existing call sites have been updated accordingly to avoid undefined
or incorrect usage.
10-12
: Avoid capitalizing non-class variables.
This mirrors a prior suggestion: consider renaming ImportContent
to a lowercase format (e.g. importContent
) for clarity and consistency.
14-16
: Validate or sanitize nextPage
before using it in an import statement.
Conditional import logic is powerful, but be sure you trust or sanitize nextPage
to mitigate potential security risks with dynamic imports.
168-168
: Double-check new parameter usage in genScriptByHook
.
The addition of the nextPage
parameter looks fine, but confirm all callers correctly pass it and that it’s not inadvertently omitted.
212-212
: Confirm consistent usage of nextPage
.
At line 212, genScript[GEN_SCRIPT_HOOKS.GEN_IMPORT]
now takes nextPage
. Make sure this parameter aligns with the rest of the codebase, especially in downstream calls.
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
Release Notes
New Features
Improvements
Technical Updates
The release introduces more dynamic page handling and improved preview capabilities across multiple components.