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.

Sign up for GitHub

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

Jump to bottom

Block control for adjacent blocks gets messed up when one of them is left/right aligned #4764

Closed
niranjan-uma-shankar opened this issue Jan 30, 2018 · 4 comments · Fixed by #11357
Closed

Block control for adjacent blocks gets messed up when one of them is left/right aligned #4764

niranjan-uma-shankar opened this issue Jan 30, 2018 · 4 comments · Fixed by #11357
Labels
Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Milestone
WordPress 5.0 RC

Comments

@niranjan-uma-shankar
Copy link

niranjan-uma-shankar commented Jan 30, 2018 •
edited
Loading

Issue Overview

Create two blocks one after the other, and left or right align any one of them. The other block, which is center aligned, has its block control extend over the entire area occupied by the former block. Also, the overflow More Options menu does not appear for the center aligned block. Thus, trying to delete from the More Options menu ends up deleting the left/right aligned block.

Steps to Reproduce (for bugs)

  1. Create any two blocks that can have an align option, one below the other.
  2. Left or right align either one of them
  3. Click the center aligned block. Observe the block border and block control extend over the area occupied by the other block.
  4. Try to delete the center aligned block. Hover mouse to the More options menu, and click delete. The center aligned block will not get deleted, but the other block gets deleted.
  5. This issue can be extended to multiple blocks. Create several blocks, align all except one to the left or right. Then click the center aligned block to observe the boundary for the block control.

Expected Behavior

  1. The boundary should enclose only the selected block, and not cover the other blocks.

Screenshots / Video

The gif below shows multiple adjacent blocks, and clicking the center aligned block has its block control overlay its preceding blocks.

d1

The gif below shows how it's impossible to delete the center aligned block (or perform any other operation from the More Options menu), since the menu option is activated only for the smaller, right aligned block.

d2
@jeffpaul jeffpaul added the [Type] Enhancement A suggestion for improvement. label Feb 1, 2018
@karmatosed karmatosed added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Apr 27, 2018
@mtias mtias added the Needs Testing Needs further testing to be confirmed. label Jul 17, 2018
@brandonpayton
Copy link
Member

I'm able to reproduce this.

@brandonpayton brandonpayton removed the Needs Testing Needs further testing to be confirmed. label Sep 25, 2018
@brandonpayton brandonpayton mentioned this issue Sep 25, 2018
Closed
2 tasks
@saulirajala saulirajala mentioned this issue Oct 6, 2018
Closed
@brandonpayton brandonpayton added this to the WordPress 5.0 milestone Oct 19, 2018
@brandonpayton
Copy link
Member

Adding this to the 5.0 milestone to replace a duplicate issue which was already in the milestone.

@mtias mtias added the Needs Design Feedback Needs general design feedback. label Oct 27, 2018
jasmussen pushed a commit that referenced this issue Nov 1, 2018
Try: Refactor contextual toolbar to work better with floats
56d8735 
This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.
@jasmussen jasmussen mentioned this issue Nov 1, 2018
Merged
@designsimply
Copy link
Member

Tested and confirmed using WordPress 4.9.8 and Gutenberg 4.2.0-rc.1 and Firefox 63.0 on macOS 10.13.6 that adding a right-aligned gallery block followed by a centered instagram block makes it so when you click on the instagram block the selection appears to encompass blocks—however, in my test, the "More options" menu did work properly and apply to the selected block it's just that the selection appears wrong. (33s)

Note: I also tested the PR in progress for the specific test case mentioned in this issue and found that it did solve the problem noted: #11357 (comment)

jasmussen pushed a commit that referenced this issue Nov 2, 2018
Try: Refactor contextual toolbar to work better with floats
af9abe1 
This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.
jasmussen pushed a commit that referenced this issue Nov 8, 2018
Try: Refactor contextual toolbar to work better with floats
15a5743 
This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.
jasmussen pushed a commit that referenced this issue Nov 9, 2018
Try: Refactor contextual toolbar to work better with floats
60d018a 
This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.
@jasmussen
Copy link
Contributor

Thansk for the report, @gziolo. Looks like the addition of the block type indicator regressed this further. But I will include a fix for both in #11357.

jasmussen added a commit that referenced this issue Nov 9, 2018
@jasmussen
Try: Refactor contextual toolbar to work better with floats (#11357)
4338dcf 
* Try: Refactor contextual toolbar to work better with floats

This is rather big surgery this late in the phase. There may be dragons, and it may be necessary to punt this to phase 2.

This PR seeks to fix #4764 by doing a number of things:

- Move the contextual toolbar into the block edit div, which is the one we float. This helps make it _connected_ to the content.
- Add some complex clearing rules so we avoid many of the gnarly situations where a selected block after a floated block has a weirdly tall size.
- Fixes so a paragraph block that follows a float does behave as it will on the frontend (i.e. won't clear), but also has a toolbar that is correctly positioned.

This moving around of things caused subsequent issues, which means this PR also:

- Fixes the toolbar appearance on mobile.
- Improves upon the appearance of the toolbar on floated items on mobile.
- Fixes hover label positioning, not only so they work with floats, but are positioned correctly as a result of this refactor.
- Fixes issues with wide and fullwide toolbar positioning.

* Use block navigator to select the blocks

* Remove block and inline level rules

This removes the rules that were specific to inline blocks or block level blocks and levels the playing field.

The benefit is no special rules. The downside is that a block following a float still might not perfectly match the user expectation.

This PR also fixes issues with mobile toolbars.

Finally, it makes the block toolbar not float. This rule used to be necessary, but for whatever reason it no longer appears to be.

* Fix rebase issue.

* Fix classic block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Needs Design Feedback Needs general design feedback. [Type] Bug An existing feature does not function as intended
Projects
None yet
Milestone
WordPress 5.0 RC
Development

Successfully merging a pull request may close this issue.

Try: Refactor contextual toolbar to work better with floats WordPress/gutenberg
7 participants
@karmatosed @brandonpayton @mtias @designsimply @jasmussen @niranjan-uma-shankar @jeffpaul