fix: undo or reply to save button tag display logic optimization#1185
fix: undo or reply to save button tag display logic optimization#1185hexqi merged 5 commits intoopentiny:release/v2.3.0from
Conversation
WalkthroughThis pull request refactors state management and messaging in several modules. The history functions no longer update the saved state unnecessarily. Additionally, new publish calls have been introduced in the canvas and block modules to emit a Changes
Sequence Diagram(s)sequenceDiagram
participant Canvas
participant Block
participant MessageBus
participant SaveToolbar
Canvas->>MessageBus: publish 'pageOrBlockInit' with schema
Block->>MessageBus: publish 'pageOrBlockInit' with block content
MessageBus->>SaveToolbar: deliver 'pageOrBlockInit' event
SaveToolbar->>SaveToolbar: Update saved state & perform schema comparison
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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 (
|
SonyLeo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/toolbars/redoundo/src/composable/useHistory.js (2)
95-101: Implementation looks good but needs error handling for localStorage.The new
toggleSaveBadgeStatusfunction effectively determines whether to show the save badge by comparing the current schema with the original. This addresses both issues mentioned in the PR:
- The save badge remains visible after "redo" if changes haven't been saved
- The save badge disappears after "undo" if all changes are canceled
However, consider adding error handling for localStorage parsing:
const toggleSaveBadgeStatus = () => { const { setSaved, getSchemaDiff } = useCanvas() - const initSchema = JSON.parse(localStorage.getItem(constants.PAGE_ORIGIN_SCHEMA)) + let initSchema + try { + initSchema = JSON.parse(localStorage.getItem(constants.PAGE_ORIGIN_SCHEMA)) + } catch (error) { + initSchema = null + } const diffContent = initSchema && getSchemaDiff(initSchema) setSaved(!diffContent) }
95-132: Consider adding documentation for the new functionality.The implementation itself is solid, but adding some comments explaining the purpose of the
toggleSaveBadgeStatusfunction and how it addresses the save badge issues would help future developers understand this code better.+/** + * Toggles the save badge status based on whether there are differences + * between the current schema and the original schema from localStorage. + * - Shows the badge when there are unsaved changes + * - Hides the badge when there are no differences (i.e., all changes are saved or undone) + */ const toggleSaveBadgeStatus = () => { const { setSaved, getSchemaDiff } = useCanvas() const initSchema = JSON.parse(localStorage.getItem(constants.PAGE_ORIGIN_SCHEMA)) const diffContent = initSchema && getSchemaDiff(initSchema) setSaved(!diffContent) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/canvas/render/src/material-function/page-getter.ts(2 hunks)packages/toolbars/redoundo/package.json(1 hunks)packages/toolbars/redoundo/src/composable/useHistory.js(3 hunks)packages/toolbars/save/src/js/index.js(1 hunks)packages/utils/src/constants/index.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (8)
packages/utils/src/constants/index.js (1)
104-104: Well-structured constant addition for schema persistence.This new constant will be used to store the original page schema in local storage, providing a reference point to determine if the page has unsaved changes. This is essential for fixing the save badge visibility issues described in the PR objectives.
packages/toolbars/redoundo/package.json (1)
29-30: Appropriate dependency addition.Adding the
@opentiny/tiny-engine-utilsdependency allows access to the constants needed for the save badge functionality fix. This change aligns perfectly with the PR objectives.packages/toolbars/save/src/js/index.js (1)
74-74: Key implementation for save badge persistence.This line saves the page schema to local storage when not in block mode, which is crucial for tracking unsaved changes. When comparing this stored schema with the current one, the application can accurately determine whether to show the save badge after redo/undo operations.
This directly addresses the PR objective of ensuring the save badge remains visible after redo operations when there are unsaved changes.
packages/canvas/render/src/material-function/page-getter.ts (2)
2-2: Appropriate utility import for constants access.This import provides access to the constants needed for consistent schema storage across different parts of the application.
36-36: Crucial implementation for schema persistence on page load.This line stores the original page schema in local storage whenever a page is fetched, establishing a baseline for comparison. This is essential for correctly determining when the save badge should appear or disappear during undo/redo operations.
This directly addresses the PR objective of ensuring the save badge disappears when undoing to the original state and remains visible when redoing changes.
packages/toolbars/redoundo/src/composable/useHistory.js (3)
14-14: Great addition of the constants import.This import allows for consistent usage of the
PAGE_ORIGIN_SCHEMAconstant across the codebase, reducing the risk of string literal mismatches.
104-104: Good placement of the toggle function call.Adding
toggleSaveBadgeStatus()at the beginning of theaddHistoryfunction ensures that the save badge status is updated whenever history is modified.
118-118: Excellent implementation for monitoring history changes.Adding the
toggleSaveBadgeStatus()call in the watch function ensures the save badge is properly updated when navigating through history (undo/redo operations). This directly addresses the issues mentioned in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/toolbars/save/src/Main.vue (2)
109-114: Well-structured state management setup.Good approach to extract additional methods from
useCanvas()and set up the message subscription system. Usingref(null)fororiginSchemais appropriate for tracking the initial schema state.Note: Consider using
const originSchema = ref(null)instead ofletsince you're not reassigning the ref itself, only its value.
116-150: Improve variable naming in schema comparison logic.The variable name
hasChangeis misleading - when the comparison returnstrue, it means there are NO changes (schemas are identical). This might cause confusion for other developers.- const hasChange = JSON.stringify(getSchema()) === originSchema.value - setSaved(hasChange) + const isUnchanged = JSON.stringify(getSchema()) === originSchema.value + setSaved(isUnchanged)Additionally, consider extracting the duplicate comparison logic from the
schemaChangeandschemaImportcallbacks into a single function to improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/toolbars/save/src/Main.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/toolbars/save/src/Main.vue (3)
61-61: Good import addition.Adding
useMessageis appropriate for implementing the subscription-based approach to track save state changes.
132-135: The fix aligns with PR objectives.This logic correctly addresses the "undo/redo" save badge issues mentioned in the PR objectives by properly tracking schema changes and updating the saved state.
152-156: Good cleanup of subscriptions.Properly unsubscribing from all topics prevents memory leaks and ensures that the event handlers won't be called after the component is unmounted.
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
Refactor