Block Navigation List: do not show appender and avoid closing the modal on block select #34337
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
…al on block select
gwwar
added
[Type] Enhancement
|
I'm also interested to also make sure the modal doesn't change in height when we drag, but I'll see if there's a reasonable way to measure the portal content. Edit: done in 8d4838d |
|
Size Change: +82 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
vcanales
approved these changes
Aug 26, 2021
There was a problem hiding this comment.
Tested, looks good to me! 🚀
Not sure if that's intentional/desirable or not, but I notice that the Navigation Block's Toolbar also hides when dragging. It makes sense that it would, since it's not even possible to use it while editing from the List View, but I find it a bit distracting regardless; perhaps worth looking at in this context, or as a follow up?
@vcanales do you have a quick screenshot or video of what you mean? I'm happy to open another issue if it doesn't fit here. |
Yes! Just referring to the toolbar for the nav block, in the background. Dragging a selected block, the toolbar for that block disappears. In this case I suppose that it makes more sense that the toolbar would go away. We could also consider selecting the parent Navigation block while dragging one of its children.
Dragging with the parten navigation block selected, the navigation block's toolbar also disappears. Here, I'm not sure that it makes a whole lot of sense for the Toolbar to disappear, as we're still working within the Navigation Block.
|
|
Oh gotcha @vcanales thanks for adding videos! Things moving around in the editor background does make it feel busy, though it's a side effect of how the dragging / global actions are dispatched. 🤔 I'm curious what others would ideally want here. Let's check in with folks and add a follow up issue. Note that if changes in #34022 eventually land, some of this is avoided since the dragging implementation uses component state and only dispatches on drop. |
|
The changes here look great 👌 I notably appreciate that the height no longer shifts. I believe that a next step after this PR could be an "add block" button at the bottom, which leverages some of the principles outlined in #34041, and we'll be in a vastly better place. Let me know if you're looking for any specific feedback! 🚀 |
|
Thanks for the reviews @vcanales @jasmussen. I'll go ahead and land this one, but happy to follow up if we need any more changes to this one. |
Closed
2 tasks
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.
This PR includes some small quality of life tweaks to the block navigation list modal. Changes here include:
After
navigation.mp4
no.shifting.mp4
Before
navb.mp4
modal.shifting.mp4
Testing Instructions