Grid View Modal

[swissspidy]

Summary

Implements the reorder grid view using react-modal.

It is still a bit rough and needs some polishing, but generally works.

Fixes #3836.

Checklist

• My pull request is addressing an open issue (please create one otherwise).
• My code is tested and passes existing tests.
• My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[miina]

Looks good and works well, left some minor comments (in addition to the mentioned SVG-s not being visible when zooming to 80% and the dropzone highlighting not being in the middle)

[swissspidy]

@miina Any suggestion for the dropzone highlighting?

I think for true responsiveness we have to use grids here, which makes using a pseudo element a bit unreliable due to unknown width of the gaps.

[miina]

@miina Any suggestion for the dropzone highlighting?
I think for true responsiveness we have to use grids here, which makes using a pseudo element a bit unreliable due to unknown width of the gaps.

In this case, isn't the grid-gap set? Could we perhaps pass it as a param to the DropZone?
Originally I was thinking that the DropZone could be used anywhere (e.g. around a Page as well to drop Media or in the Media library to drop Media), and the styling could be passed as attributes from each implementation as necessary.

I'm not sure though, what did you have in mind?

[swissspidy]

Right, yeah grid-gap is set. Passing it as a param makes sense. For the carousel it would be the margin, here the grid-gap.

[swissspidy]

@miina Implemented that in b98129d. WDYT?

[miina]

@miina Implemented that in b98129d. WDYT?

Seems to work well.

The only thing I'm noticing now is that the highlights seem to be lagging on the grid view, can you also see that? Looking at the code then it seems like the issue is coming from dropZoneProvider where setHoveredDropZone is done even if all the params are the same.

E.g. adding this removes the lagging:

if (
			! hoveredDropZone ||
			hoveredDropZone.node !== foundDropZone.node ||
			position.x !== hoveredDropZone.position.x ||
			position.y !== hoveredDropZone.position.y
		) {
			setHoveredDropZone( {
				node: foundDropZone.node,
				position,
			} );
		}

Although the issue doesn't come from this PR, would you mind adjusting this here, too?

[swissspidy]

Sure thing, done! Definitely improves things.

[miina]

LGTM