-
Notifications
You must be signed in to change notification settings - Fork 342
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
fix(canvas/render): fix edited page not refresh on children page, fix pageSection should not be able to drag,remove post css warning #1060
Conversation
…t but should not be able to drag
WalkthroughThis pull request introduces modifications to several files in the canvas rendering system, focusing on enhancing component type handling and state management. The changes include adding a Changes
Sequence DiagramsequenceDiagram
participant Renderer as Custom Renderer
participant PageGetter as Page Getter
participant RenderLogic as Render Logic
Renderer->>PageGetter: Monitor component active state
alt Component becomes inactive
PageGetter->>PageGetter: Clear async data
end
RenderLogic->>RenderLogic: Process component type
RenderLogic->>RenderLogic: Determine draggability
Possibly related PRs
Suggested Labels
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 (
|
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 (2)
packages/canvas/render/src/material-function/page-getter.ts (1)
53-54
: LGTM! Core fix for stale data issueSetting
asyncData.value = null
when the page becomes inactive ensures that stale data is cleared, directly addressing the PR's main objective of fixing refresh issues on child pages.Consider optimizing the update logic
The immediate call to
updateSchema()
after nullifying the data might be unnecessary since the data will be fetched again when the page becomes active.if (!activeValue) { asyncData.value = null - updateSchema() }
packages/canvas/render/src/render.ts (1)
Line range hint
126-129
: Consider performance optimization for property deletionThe static analysis tool flagged the use of the
delete
operator. While valid in this context, consider using undefined assignment for better performance.- invalidity.forEach((prop) => delete bindProps[prop]) + invalidity.forEach((prop) => { bindProps[prop] = undefined })🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/canvas/render/src/canvas-function/custom-renderer.ts
(1 hunks)packages/canvas/render/src/material-function/handle-scoped-css.ts
(1 hunks)packages/canvas/render/src/material-function/page-getter.ts
(1 hunks)packages/canvas/render/src/render.ts
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/canvas/render/src/render.ts
[error] 123-123: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (4)
packages/canvas/render/src/material-function/handle-scoped-css.ts (1)
5-5
: LGTM! PostCSS warning fixAdding
{ from: undefined }
is the correct way to suppress PostCSS source file warnings.packages/canvas/render/src/canvas-function/custom-renderer.ts (1)
10-10
: LGTM! Component type additionAdding
componentType: 'PageSection'
correctly identifies the root container and prevents it from being draggable, which aligns with the PR objective to fix drag-and-drop related issues.Let's verify that this component type is consistently used:
✅ Verification successful
✓ Component type usage is consistent
The
componentType: 'PageSection'
is correctly used as a unique identifier for the root container, working alongside other page-related component types likePageStart
. No inconsistencies found in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other PageSection type declarations to ensure consistency rg "componentType.*PageSection" -g "!*.{json,md}"Length of output: 147
Script:
#!/bin/bash # Search for similar section/page component types rg "componentType.*[Ss]ection|componentType.*[Pp]age" -g "!*.{json,md}" # Search for PageSection in type definitions and interfaces ast-grep --pattern 'interface $_ { $$$ PageSection $$$ }' ast-grep --pattern 'type $_ = { $$$ PageSection $$$ }' # Search for PageSection in test files rg "PageSection" -g "*.test.{ts,tsx,js,jsx}"Length of output: 15968
packages/canvas/render/src/render.ts (2)
85-85
: LGTM! Added componentType to schema destructuringCorrectly extracts componentType for use in drag-and-drop logic.
123-125
: LGTM! Improved drag-and-drop controlThe condition
!['PageStart', 'PageSection'].includes(componentType)
correctly prevents dragging of page-level components, which aligns with the PR objective to fix drag-and-drop related issues.🧰 Tools
🪛 Biome (1.9.4)
[error] 123-123: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
… pageSection should not be able to drag,remove post css warning (opentiny#1060)
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
Enhancements
Technical Improvements