feat(canvas/container): canvas allow router-link quick switch page#1001
Conversation
…ive page to have hover menu for quick switch page action
|
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 a new feature for handling inactive elements in the canvas system. A new constant Changes
Sequence DiagramsequenceDiagram
participant Canvas as CanvasContainer
participant Action as CanvasAction
participant Jumper as CanvasRouterJumper
participant Render as Canvas Render
Canvas->>Action: Pass inactiveHoverState
Action-->>Jumper: Render inactive hover state
Jumper->>Render: Update inactive element properties
Render-->>Canvas: Reflect inactive element state
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
packages/canvas/container/src/CanvasContainer.vue (1)
41-41: Consider adding TypeScript type definitions.While the implementation is correct, adding TypeScript interfaces for the component props and state would improve type safety and developer experience.
interface HoverState { componentName?: string; element?: HTMLElement; width?: number; left?: number; top?: number; } interface Props { controller: object; canvasSrc?: string; canvasSrcDoc?: string; materialsPanel?: object; }Also applies to: 67-67, 254-254
packages/canvas/container/src/components/CanvasRouterJumper.vue (1)
77-107: Enhance accessibility and maintainability of styles.Consider these improvements:
- Extract magic numbers to CSS variables
- Add ARIA attributes for better accessibility
<template> <div v-if="state.showRouterJumper" + role="button" + aria-label="Quick page navigation" :class="{ 'jumper-wrapper': true, disabled: !state.targetPageId }" :title="state.targetPageId ? '路由跳转编辑' : '未绑定路由跳转页面'" @click="routerPageJump" ><style lang="less" scoped> +:root { + --jumper-size: 24px; + --jumper-icon-size: 16px; + --jumper-transform-x: -80%; + --jumper-transform-y: -20%; +} .jumper-wrapper { position: absolute; display: flex; align-items: center; justify-content: center; - width: 24px; - height: 24px; + width: var(--jumper-size); + height: var(--jumper-size); border-radius: 50%; background-color: var(--ti-lowcode-common-text-color-3); cursor: pointer; z-index: 3; - transform: translateX(-80%) translateY(-20%); + transform: translateX(var(--jumper-transform-x)) translateY(var(--jumper-transform-y)); top: v-bind('state.top'); left: v-bind('state.left'); border: 1px solid var(--te-common-border-hover); &.disabled { opacity: 0.3; } &:not(.disabled):hover { border-color: var(--te-common-bg-primary-checked); background-color: var(--te-common-bg-primary-checked); .jumper { color: var(--te-common-text-dark-inverse); } } .jumper { - width: 16px; - height: 16px; + width: var(--jumper-icon-size); + height: var(--jumper-icon-size); } } </style>packages/canvas/container/src/components/CanvasAction.vue (1)
547-562: Consider using CSS custom properties for colors.The inactive hover state styles are well-structured, but consider extracting the color values into CSS custom properties for better maintainability and theming support.
&.inactive-hover { border-style: dashed; top: v-bind("inactiveHoverState.top + 'px'"); left: v-bind("inactiveHoverState.left + 'px'"); height: v-bind("inactiveHoverState.height + 'px'"); width: v-bind("inactiveHoverState.width + 'px'"); - border-color: var(--te-common-border-hover); + border-color: var(--ti-lowcode-canvas-inactive-hover-border-color, var(--te-common-border-hover)); .corner-mark-left { height: 14px; top: -14px; padding-left: 0; font-size: 12px; - color: var(--te-common-text-weaken); + color: var(--ti-lowcode-canvas-inactive-hover-text-color, var(--te-common-text-weaken)); } }packages/canvas/container/src/container.js (2)
242-268: Consider adding JSDoc documentation for getInactiveElement.The function is well-implemented but lacks documentation explaining its purpose and return values.
+/** + * Gets the closest ancestor element that has a NODE_INACTIVE_UID attribute. + * @param {HTMLElement} element - The starting element to search from. + * @returns {HTMLElement|undefined} The found element or undefined if no matching element is found. + */ export const getInactiveElement = (element) => { // ... existing implementation }
585-608: Consider adding error handling for invalid element.The function should handle cases where the element's attributes are undefined.
const setInactiveHoverRect = (element) => { if (!element) { Object.assign(inactiveHoverState, initialRectState, { slot: null }) return } + const nodeTag = element.getAttribute(NODE_TAG) + const nodeId = element.getAttribute(NODE_INACTIVE_UID) + + if (!nodeTag || !nodeId) { + Object.assign(inactiveHoverState, initialRectState, { slot: null }) + return + } - const componentName = element.getAttribute(NODE_TAG) - const id = element.getAttribute(NODE_INACTIVE_UID) + const componentName = nodeTag + const id = nodeId const configure = getConfigure(componentName) // ... rest of the implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/design-core/assets/jump.svgis excluded by!**/*.svg
📒 Files selected for processing (7)
packages/canvas/common/src/constant.js(1 hunks)packages/canvas/container/src/CanvasContainer.vue(5 hunks)packages/canvas/container/src/components/CanvasAction.vue(3 hunks)packages/canvas/container/src/components/CanvasRouterJumper.vue(1 hunks)packages/canvas/container/src/container.js(5 hunks)packages/canvas/render/src/builtin/CanvasRouterLink.vue(1 hunks)packages/canvas/render/src/render.ts(2 hunks)
🔇 Additional comments (10)
packages/canvas/common/src/constant.js (1)
4-4: LGTM! Well-defined constant.The constant follows the established naming pattern and HTML5 data-* attribute convention.
packages/canvas/render/src/render.ts (2)
15-20: LGTM! Clean import organization.The import statement is well-organized and groups related constants together.
96-96: LGTM! Robust state handling.The ternary operator elegantly handles both active and inactive states while maintaining the element's identity through appropriate data attributes.
packages/canvas/container/src/CanvasContainer.vue (1)
4-4: LGTM! Clean component integration.The new router jumper component is well-integrated with proper prop passing.
Also applies to: 12-12
packages/canvas/container/src/components/CanvasRouterJumper.vue (1)
23-33: LGTM! Well-structured props definition.Props are properly defined with appropriate types and default values.
packages/canvas/container/src/components/CanvasAction.vue (2)
102-106: LGTM! The inactive hover state template is well-structured.The template follows the same pattern as the existing hover state, providing consistent user experience.
173-176: LGTM! The inactiveHoverState prop is properly defined.The prop type and default value are correctly specified, matching the pattern used for other state props.
packages/canvas/container/src/container.js (2)
20-21: LGTM! Constants are properly imported.The new constants are imported alongside existing ones, maintaining code organization.
123-125: LGTM! The inactive hover state is properly initialized.The state follows the same pattern as other reactive states, using the initialRectState.
packages/canvas/render/src/builtin/CanvasRouterLink.vue (1)
4-4: LGTM! The data attribute binding is properly implemented.The data attribute binding uses optional chaining to safely access the
to.nameproperty.
3 participants
…tive page to have hover menu for quick switch page action
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?
跳转编辑不方便，尤其如果RouterLink处于非激活页面就无法选中右键跳转
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
CanvasRouterJumpercomponent for enhanced routing interactionsImprovements
Technical Updates