fix: parent form field in page settings do not display #1069
Conversation
WalkthroughThe pull request focuses on modifying the page management logic in the Vue.js application. Changes are made to Changes
Sequence DiagramsequenceDiagram
participant Vue Component
participant usePage
participant TreeData
Vue Component->>usePage: Call changeTreeData
usePage->>usePage: getParentNode(parentId)
usePage->>TreeData: Retrieve parent node
usePage->>TreeData: Update tree structure
TreeData-->>usePage: Return updated data
usePage-->>Vue Component: Confirm changes
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/plugins/page/src/PageGeneral.vue (1)
35-35: Consider renaming the event handler to improve clarity.
The method namechangeParentForderIdlikely contains a typo and could be renamed to something likechangeParentFolderIdfor better readability.- @change="changeParentForderId" + @change="changeParentFolderId"packages/plugins/page/src/composable/usePage.js (1)
78-80: Consider naming strategy for group IDs.
Hardcoding these numeric IDs is fine for now, but if more groups are introduced, you may want a more scalable naming or an enum-like structure for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/page/src/PageGeneral.vue(3 hunks)packages/plugins/page/src/composable/usePage.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/plugins/page/src/PageGeneral.vue (3)
97-105: Validate edge cases when forcing the parentId to a string.
WrappingpageSettingState.currentPageData.parentIdinString()ensures consistent string-based comparisons; however, be aware thatnull,undefined, or other falsy values will become "null" or "undefined" strings. If that’s unintended, consider guarding against these values.
115-115: Check for potential infinite loops or missing entries in treeDataMapping.
While this loop breaks ifparentis falsy, ensureparentIdalways moves closer toROOT_ID. Any unexpected missing entries or accidental cyclical references could cause issues.
246-246: Good approach exposing pageParentId.
Returning the computed property makes the parent ID logic self-contained and clearly managed within the component.packages/plugins/page/src/composable/usePage.js (1)
157-162: Handle nonexistent entries in treeDataMapping.
getParentNodemay returnundefinediftreeDataMappinglacks the given parent key. If that scenario is possible, consider returning a fallback or throwing an error.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/plugins/page/src/composable/usePage.js (2)
157-161: Add JSDoc documentation and type safety.The function logic is correct, but could benefit from improved documentation and type safety.
+/** + * Retrieves the parent node based on the provided parentId + * @param {string | number} parentId - The ID of the parent node + * @returns {PageNode | undefined} The parent node or undefined if not found + */ const getParentNode = (parentId) => { + if (typeof parentId !== 'string' && typeof parentId !== 'number') { + return undefined + } return parentId === pageSettingState.ROOT_ID ? { id: pageSettingState.ROOT_ID, children: pageSettingState.pages[STATIC_PAGE_GROUP_ID].data } : pageSettingState.treeDataMapping[parentId] }
164-171: Add error handling and return value.While the null checks are good, consider adding error handling and documenting the return value.
+/** + * Changes the tree data structure when a node's parent changes + * @param {string | number} newParentId - The ID of the new parent + * @param {string | number} oldParentId - The ID of the old parent + * @returns {boolean} True if the change was successful, false otherwise + */ const changeTreeData = (newParentId, oldParentId) => { if (newParentId && oldParentId && String(newParentId) !== String(oldParentId)) { const folderData = getParentNode(newParentId) const parentData = getParentNode(oldParentId) if (!folderData || !parentData) { - return + return false } const currentPageDataId = pageSettingState.currentPageData.id const curDataIndex = parentData.children?.findIndex?.(({ id }) => id === currentPageDataId) if (curDataIndex > -1) { const splicedPageData = parentData.children.splice(curDataIndex, 1)[0] if (!folderData.children) { folderData.children = [] } folderData.children.unshift(splicedPageData) + return true } } + return false }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/page/src/composable/usePage.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/plugins/page/src/composable/usePage.js (1)
78-80: LGTM! Good use of constants for page group IDs.The constants improve code maintainability by replacing magic numbers and are used consistently throughout the code.
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?
页面新建子页面或者子文件夹，父没有正确显示
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Refactor
Bug Fixes