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

Fix spacer block resizing #7799

Merged
merged 1 commit into from
Jul 11, 2018
Merged

Fix spacer block resizing #7799

merged 1 commit into from
Jul 11, 2018

Conversation

noisysocks
Copy link
Member

@noisysocks noisysocks commented Jul 9, 2018

Fixes #7755.

The sibling inserter was preventing a user from being able to resize the spacer using the top drag handle.

spacer-resizing

To test:

  1. Insert a spacer block
  2. Check that it can be resized using both drag handles
  3. Insert an image block
  4. Check that it can be resized using both drag handles

@noisysocks noisysocks added [Type] Bug An existing feature does not function as intended [Feature] Blocks Overall functionality of blocks labels Jul 9, 2018
@noisysocks noisysocks added this to the 3.3 milestone Jul 9, 2018
@noisysocks noisysocks requested a review from mcsf July 9, 2018 06:48
@mcsf
Copy link
Contributor

mcsf commented Jul 9, 2018

Thanks for looking into this. Unfortunately, this way, one can't get to the upper sibling inserter when selecting a Spacer block.

P.S.: Edited the PR description to fix the "Fixes #xxxx" against the correct issue.

@ZebulanStanphill
Copy link
Member

Would it make more sense to just remove the drag handle from the top of the Spacer block? There is not really a need for one on both the top and bottom, and dragging from the top works a little oddly.

@mcsf
Copy link
Contributor

mcsf commented Jul 9, 2018

cc @jasmussen

The top resize handle and the sibling inserter are in the same spot.
Removing the top resize handle is a simple fix.
@noisysocks noisysocks force-pushed the fix/spacer-resizing branch from d7c844e to 138cda9 Compare July 10, 2018 00:26
@noisysocks
Copy link
Member Author

Doh, I totally missed that. 🤦‍♂️

I've updated this PR to remove the top resize handle altogether:

spacer-resizing

@mcsf
Copy link
Contributor

mcsf commented Jul 10, 2018

@noisysocks: thanks, though the upper sibling inserter is still inaccessible. :)

@noisysocks
Copy link
Member Author

🤔 perhaps this is intentional? @jasmussen?

It looks like, in master, the sibling inserter never shows when you hover over the top of the selected block, unless Fix Toolbar to Top is enabled.

sibling-inserter

@jasmussen
Copy link
Contributor

jasmussen commented Jul 11, 2018

This is intentional for the sibling inserter when a block is selected and that block has no toolbar to cover the sibling inserter.

It seems this aspect is non optimal when a block has no toolbar, as in the case of the spacer. As we look for a way to fix this let's be sure to not regress this sibling inserter behavior for blocks with toolbars.

@jasmussen
Copy link
Contributor

And by the way I thing it's fine to simply not show the top resizing handle. Your mouse is floating in air anyway, when you use it.

@noisysocks
Copy link
Member Author

I think then let's leave this as is. We can look at making the sibling inserter appear when the block has no toolbar in a seperate PR. Issue here: #7884

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

@mtias mtias merged commit 993f890 into master Jul 11, 2018
@mtias mtias deleted the fix/spacer-resizing branch July 11, 2018 10:20
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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants