-
Notifications
You must be signed in to change notification settings - Fork 454
feat:preview multi page structured data transfer and nested display #978
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
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 replacingRouterViewis clever; handle multi-view scenarios.Replacing
'RouterView'withnextPagehelps in dynamic page routing, but watch for edge-cases like multipleRouterViewinstances in a single layout. Consider how the logic should behave if multiple views exist or ifcomponentNamemight need further checks.packages/toolbars/preview/src/Main.vue (1)
33-33: DestructuregetFamilyfor clarity.Nice extraction of
getFamilyfromusePage. 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: ForwardnextPageinto template generation hooks.Passing
nextPagetogenTemplateByHookcan 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: StandalonegetAncestorsRecursivelyfunction 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:getFamilyfilters pages only, reversing for a top-down order.That’s a straightforward approach. It might be worth adding a default fallback if
idis 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.