Move selected node state to page view only #1679
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apedroferreira
added
core
oliviertassinari
requested a deployment
to
move-selected-node-to-page-view - toolpad-db PR #1679
Render
Abandoned
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
github-actions
bot
added
the
PR: out-of-date
apedroferreira
force-pushed
the
move-selected-node-to-page-view
branch
from
b217123
to
d315214
Compare
oliviertassinari
requested a deployment
to
move-selected-node-to-page-view - toolpad-db PR #1679
Render
Abandoned
github-actions
bot
removed
the
PR: out-of-date
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
|
I moved |
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
oliviertassinari
temporarily deployed
to
move-selected-node-to-page-view - toolpad PR #1679
Render
Destroyed
Janpot
approved these changes
Feb 21, 2023
| @@ -101,10 +101,8 @@ export default function CreateCodeComponentDialog({ | |||
| const appNode = appDom.getApp(dom); | |||
|
|
|||
| appStateApi.update((draft) => appDom.addNode(draft, newNode, appNode, 'codeComponents'), { | |||
There was a problem hiding this comment.
Not introduced in this PR, but is there any technical reason why update updates both the dom and the view? It feels like two different things to me
There was a problem hiding this comment.
In some cases for the undo/redo history to work well the best is that we update the DOM and the view in the same action, especially when adding or removing nodes. Otherwise the user might end up, for example, in a page view for a page node that has been deleted. Maybe there's a better alternative though?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move
selectedNodeIdandcurrentTabstate to views ofkind: 'page', as that's the only kind of view where it makes sense to keep that state.Also deselects node when switching to a different page or view.
As suggested in #1672 (comment)