Block Popover: editor canvas as boundary #19322

ellatrix · 2019-12-24T12:55:05Z

Description

This is just a proof of concept. I wouldn't recommend using it on its own as it may look a bit strange with the currently block selection style and the movers on the side.

Eventually we should rewrite Popover (or use an external component), but I'm trying to avoid that here so it unblocks other work being don.

How has this been tested?

Screenshots

Types of changes

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.
I've updated all React Native files affected by any refactorings/renamings in this PR. .

ellatrix · 2019-12-24T13:04:36Z

packages/components/src/popover/utils.js

@@ -45,7 +46,7 @@ export function computePopoverXAxisPosition( anchorRect, contentSize, xAxis, cor
 		popoverLeft: anchorMidPoint,
 		contentWidth: (
 			( anchorMidPoint - ( width / 2 ) > 0 ? ( width / 2 ) : anchorMidPoint ) +
-			( anchorMidPoint + ( width / 2 ) > window.innerWidth ? window.innerWidth - anchorMidPoint : ( width / 2 ) )
+			( anchorMidPoint + ( width / 2 ) > boundaryElement.clientWidth ? boundaryElement.clientWidth - anchorMidPoint : ( width / 2 ) )


We'd need to test really long toolbars. Maybe this can be done with CSS though, if the popover is positioned in the boundary element?

ZebulanStanphill · 2019-12-24T18:48:50Z

The navigation mode toolbar is currently positioned exactly the same as the edit mode toolbar, which results in some weirdness in situations like this:

It is also notable that the block mover gets pushed to the left as well, which is not ideal. I think this should probably land around the same time as #18685.

Also, the heavy left border on the toolbar no longer fits when the toolbar gets pushed to the left.

Aside from those issues, this is a great improvement over master. Editing nested blocks close to the right edge of the screen is a lot easier now.

ellatrix · 2019-12-25T19:46:39Z

@ZebulanStanphill this PR is meant to go together with the block mover PR, and it’s not finished. :) @youknowriad feel free to take it and change within your PR

jasmussen · 2020-01-13T09:52:12Z

This is so good, thank you for working on this.

One of the concepts being explored in #18667 is to offset the toolbar to the left of the block. Like so:

Additionally we are exploring how to surface the block mover only when it's necessary. One early idea is to show those on hover or focus in the toolbar:

<img src="https://user-images.githubusercontent.com/548849/69341684-4ca26800-0c6a-11ea-88f1-b9eb44a3a96e.gif"

This mover control popping out is obviously going to make the horizontal positioning somewhat more complex, given it makes it wider. Is this something that this PR in question can handle?

ellatrix · 2020-01-13T15:28:08Z

This PR is blocked by #19596. I rebased this PR to that branch.

ellatrix · 2020-01-13T16:27:43Z

Right. The movers are positioned relative to the block toolbar. We could also split it and use two popovers, but I think it's not worth spending time on it if they'll eventually move back too the toolbar.

youknowriad · 2020-01-14T09:19:05Z

packages/block-editor/src/components/block-list/block-popover.js

@@ -124,10 +124,14 @@ function BlockPopover( {
 			className="block-editor-block-list__block-popover"
 			__unstableSticky={ showEmptyBlockSideInserter ? false : popoverIsSticky }
 			__unstableSlotName="block-toolbar"
+			__unstableBoundaryParent
 			// Allow subpixel positioning for the block movement animation.
 			__unstableAllowVerticalSubpixelPosition={ moverDirection !== 'horizontal' && node }
 			__unstableAllowHorizontalSubpixelPosition={ moverDirection === 'horizontal' && node }


It feels like we have too many unstable props for the Popover

Yeah 😅 Soon we should look into rewriting Popover or swapping it out, but I'd like to wait for some more simplifications and use cases for positioning.

youknowriad

should we merge this into #18685 ?

ellatrix · 2020-01-14T09:37:16Z

I changed the merge base, but I think that PR will need a rebase.

youknowriad · 2020-01-14T09:43:59Z

I rebased it.

ellatrix added the [Status] In Progress Tracking issues with work in progress label Dec 24, 2019

ellatrix requested review from ajitbohra, chrisvanpatten, jorgefilipecosta, talldan and youknowriad as code owners December 24, 2019 12:55

ellatrix commented Dec 24, 2019

View reviewed changes

This was referenced Jan 13, 2020

Block: remove tabbable inserter button #19594

Closed

Block: reposition tabbable inserter #19596

Merged

ellatrix changed the base branch from master to try/reposition-tabbable-inserter January 13, 2020 15:25

ellatrix force-pushed the try/popover-boundary branch from ffff29d to 7c2efe7 Compare January 13, 2020 15:47

ellatrix changed the title ~~[WIP] Popover: different boundary element~~ Block Popover: editor canvas as boundary Jan 13, 2020

ellatrix removed the [Status] In Progress Tracking issues with work in progress label Jan 13, 2020

ellatrix added 4 commits January 13, 2020 17:14

Popover: different boundary element

60530f5

Use slot parent elementt

70e4b72

Revert width/height calc

d10d193

Fix breadcrumb position

9812fe6

ellatrix force-pushed the try/popover-boundary branch from bce6e01 to 9812fe6 Compare January 13, 2020 16:14

ellatrix requested review from adamsilverstein, aduth, atimmer, daniloercoli, etoledom, gziolo, mmtr and nerrad as code owners January 13, 2020 16:14

youknowriad reviewed Jan 14, 2020

View reviewed changes

youknowriad approved these changes Jan 14, 2020

View reviewed changes

ellatrix changed the base branch from master to try/move-movers-to-toolbar January 14, 2020 09:36

youknowriad force-pushed the try/move-movers-to-toolbar branch from 2d7bb30 to c25b93e Compare January 14, 2020 09:43

youknowriad merged commit 650b277 into try/move-movers-to-toolbar Jan 14, 2020

youknowriad deleted the try/popover-boundary branch January 14, 2020 09:44

youknowriad mentioned this pull request Jan 14, 2020

Move the block movers to the block toolbar #18685

Closed

jasmussen pushed a commit that referenced this pull request Jan 14, 2020

Block Popover: editor canvas as boundary (#19322)

0449a2b

youknowriad pushed a commit that referenced this pull request Jan 16, 2020

Block Popover: editor canvas as boundary (#19322)

e8c3b8f

ellatrix added this to the Gutenberg 7.3 milestone Jan 20, 2020

jasmussen pushed a commit that referenced this pull request Jan 21, 2020

Block Popover: editor canvas as boundary (#19322)

04d98f8

jasmussen pushed a commit that referenced this pull request Jan 22, 2020

Block Popover: editor canvas as boundary (#19322)

8e5baec

jasmussen pushed a commit that referenced this pull request Jan 23, 2020

Block Popover: editor canvas as boundary (#19322)

3c58cbb

youknowriad pushed a commit that referenced this pull request Feb 4, 2020

Block Popover: editor canvas as boundary (#19322)

72a9994

jasmussen pushed a commit that referenced this pull request Feb 6, 2020

Block Popover: editor canvas as boundary (#19322)

ccfaa77

jasmussen pushed a commit that referenced this pull request Feb 11, 2020

Block Popover: editor canvas as boundary (#19322)

e08a4c6

youknowriad pushed a commit that referenced this pull request Feb 13, 2020

Block Popover: editor canvas as boundary (#19322)

d6b7668

jasmussen pushed a commit that referenced this pull request Feb 18, 2020

Block Popover: editor canvas as boundary (#19322)

42c9c58

jasmussen pushed a commit that referenced this pull request Feb 18, 2020

Block Popover: editor canvas as boundary (#19322)

94cb5d9

youknowriad pushed a commit that referenced this pull request Feb 24, 2020

Block Popover: editor canvas as boundary (#19322)

efc9f12

youknowriad pushed a commit that referenced this pull request Feb 24, 2020

Block Popover: editor canvas as boundary (#19322)

d146aa4

sirreal mentioned this pull request May 11, 2020

Popover: Fix closest().parentNode null error #22264

Merged

6 tasks

talldan mentioned this pull request Jun 26, 2020

Try removing BlockPopover key and related code. #23504

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Block Popover: editor canvas as boundary #19322

Block Popover: editor canvas as boundary #19322

ellatrix commented Dec 24, 2019 •

edited

Loading

ellatrix Dec 24, 2019

ZebulanStanphill commented Dec 24, 2019

ellatrix commented Dec 25, 2019

jasmussen commented Jan 13, 2020

ellatrix commented Jan 13, 2020

ellatrix commented Jan 13, 2020

youknowriad Jan 14, 2020

ellatrix Jan 14, 2020

youknowriad left a comment

ellatrix commented Jan 14, 2020

youknowriad commented Jan 14, 2020

Block Popover: editor canvas as boundary #19322

Block Popover: editor canvas as boundary #19322

Conversation

ellatrix commented Dec 24, 2019 • edited Loading

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

ellatrix Dec 24, 2019

Choose a reason for hiding this comment

ZebulanStanphill commented Dec 24, 2019

ellatrix commented Dec 25, 2019

jasmussen commented Jan 13, 2020

ellatrix commented Jan 13, 2020

ellatrix commented Jan 13, 2020

youknowriad Jan 14, 2020

Choose a reason for hiding this comment

ellatrix Jan 14, 2020

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

ellatrix commented Jan 14, 2020

youknowriad commented Jan 14, 2020

ellatrix commented Dec 24, 2019 •

edited

Loading