Skip to content
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

Block Editor: Only show block movers when block is selected. #16745

Closed

Conversation

epiqueras
Copy link
Contributor

Fixes #16681

Description

Avoids block movers overlapping when there are certain combinations of blocks, block selections, and/or hovers, by only showing them when a block is selected and not on hover.

How has this been tested?

Blocks were hovered and it was verified that the movers were not showing.

Screenshots

Screen Shot 2019-07-24 at 4 36 30 PM

Types of Changes

Bug Fix: Only show block movers when block is selected.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@epiqueras epiqueras self-assigned this Jul 24, 2019
@epiqueras epiqueras added [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks labels Jul 24, 2019
@epiqueras epiqueras added this to the Gutenberg 6.2 milestone Jul 24, 2019
@youknowriad youknowriad requested review from mtias and jasmussen July 25, 2019 09:34
@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Jul 25, 2019
@youknowriad
Copy link
Contributor

I don't have a strong preference personally but I'd love thoughts from @mtias and @jasmussen as this is one of the early decisions and we should check the reasoning behind it as it does add an extra click if you'd want to move a block before selecting it.

@gziolo gziolo removed this from the Gutenberg 6.2 milestone Jul 30, 2019
@marekhrabe
Copy link
Contributor

What if they showed on hover in case no block at all is selected? Would that be too magical?

@jasmussen
Copy link
Contributor

Thanks for this PR, thanks for taking a stab at this.

It doesn't feel like it directly addresses #16681 though, not as I perceive it. @kjellr can you confirm?

What it does is always show it when the block is selected, and remove it entirely from hover. This has been suggested and discussed in the past, and while it's definitely an option on the table, it does not seem to add that much value, whereas removing the move-on-hover feature does retract a bit. It was intentially built so that these tools were immediately available.

However on a high level, this PR does simplify things a little bit. And one of the challenges that would be good to look at, is reducing the complexity of this mover control. In the not too distant future, we are likely to see blocks that require horizontal rearrangement of child block. Think: a navigation menu where you can sort the items horizontally. It would be nice to do some "back to basics" thinking on how blocks are moved — potentially an idea is out there that allows us to both simplify this mover, and make it work in multiple directions.

@kjellr
Copy link
Contributor

kjellr commented Aug 5, 2019

It doesn't feel like it directly addresses #16681 though, not as I perceive it. @kjellr can you confirm?

I think this technically solves the problem. But it goes about it in a different way than I'd been thinking. I'd thought we should keep the current ability to show movers on non-selected blocks, but just make sure that only one instance of block movers is visible at any given time. So when a new set of block movers becomes visible, all other instances of block movers disappear.

This does make things simpler, but I do agree that it requires extra clicking to move things around. I'd probably vote to hold off for a different solution.

@epiqueras
Copy link
Contributor Author

What if we only show block movers when hovered, and not when just selected?

Otherwise we would need to hoist hover state up into a parent or into the store so that selected blocks can hide their block movers when they see that another block is hovered. Unless I missed a simpler way to do it. cc @youknowriad

@jasmussen
Copy link
Contributor

What if we only show block movers when hovered, and not when just selected?

Can you elaborate?

One of the things that happens when you click on the mover, or drag, is that the block gets selected also. So if the movers are only available on hover, it would effectively break things, if I'm understanding things correctly. Additionally we need them to be keyboard accessible.

@epiqueras
Copy link
Contributor Author

One of the things that happens when you click on the mover, or drag, is that the block gets selected also. So if the movers are only available on hover, it would effectively break things, if I'm understanding things correctly. Additionally we need them to be keyboard accessible.

You can still hover over a selected block. Basically, only show the movers when the mouse is on top of the block, regardless of whether it's selected or not.

@mapk
Copy link
Contributor

mapk commented Sep 23, 2019

Looks like this has been resolved with this duplicate: #17315

For this reason, I'm closing this PR.

@mapk mapk closed this Sep 23, 2019
@youknowriad youknowriad deleted the fix/only-show-one-set-of-block-movers-at-a-time branch September 23, 2019 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks Needs Design Feedback Needs general design feedback. [Package] Block editor /packages/block-editor [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Only show one set of block movers at a time.
7 participants