Implemented Gallery Drag and Drop Reordering #7946

caxco93 · 2018-07-13T05:54:10Z

Description

Implemented Gallery Drag and Drop Reordering
* Created new Sortable HOC for wrapping the Gallery Images
* Added react-sortable-hoc dependency

Addresses Issue #743

Screenshots

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

My code follows the WordPress code style.
My code has proper inline documentation.
My code is tested.
My code follows the accessibility standards.

KNOWN ISSUES:

* Selected image styles are not displayed properly while dragged (this is because
  react-sortable-hoc creates a copy of the dragged element right before the </body>
  tag, but for these images, the style depend on the <ul> parent)

ajitbohra · 2018-07-13T20:25:20Z

We can extract sortable-hoc as a generic independent component.

It will be a good addon to current available hoc https://github.com/WordPress/gutenberg/tree/master/components/higher-order

which are being moved to separate package compose #7948

It can be used internally and also by block developers

Something similar a hoc component using react-sortable-hoc https://github.com/lucprincen/gutenberg-sortable

caxco93 · 2018-07-17T15:03:16Z

Yeah having it as a generic independent component would be ideal. Right now it's being specific to this block behavior though. I think after its completed it can be extracted.

caxco93 · 2018-07-17T15:10:21Z

Hey everyone I was wondering whether the reordering behaviour should work with a hold press delay to start dragging, or a handle from where you can drag instantaneously. What do you think?

karmatosed · 2018-07-18T20:25:12Z

Hey everyone I was wondering whether the reordering behaviour should work with a hold press delay to start dragging, or a handle from where you can drag instantaneously. What do you think?

Why does it need to have a trigger? Can't it just work on mouse drag? If it has to have one I guess hold press but I'd love it to 'just work'.

ZebulanStanphill · 2018-07-18T20:34:39Z

Yeah, I would prefer instant drag as well, unless there is some technical issue that prevents it.

caxco93 · 2018-07-19T22:00:24Z

gotcha! i'll aim towards that behaviour then!

caxco93 · 2018-07-30T20:51:08Z

Right now im dealing with the Image Caption Focus resetting after 1 key stroke. I have just now noticed it has been caused by the HOC around the GalleryImage components.

The SortableContainer HOC seems to be always rerendering the whole thing even after 1 click or key stroke, which causes the focus to be lost

noisysocks · 2018-08-01T00:46:10Z

I'm getting an exception when I:

Insert a new gallery block
Click Media Library
Select some images, then proceed to Insert gallery

sortable.js:39 Uncaught TypeError: Cannot add property tabindex, object is not extensible
    at sortable.js:39
    at Array.map (<anonymous>)
    at sortable.js:38
    at mountIndeterminateComponent (react-dom.24169eaf.js:13958)
    at beginWork (react-dom.24169eaf.js:14398)
    at performUnitOfWork (react-dom.24169eaf.js:16441)
    at workLoop (react-dom.24169eaf.js:16480)
    at HTMLUnknownElement.callCallback (react-dom.24169eaf.js:140)
    at Object.invokeGuardedCallbackDev (react-dom.24169eaf.js:178)
    at invokeGuardedCallback (react-dom.24169eaf.js:227)

noisysocks · 2018-08-01T00:47:54Z

package.json

@@ -55,6 +55,7 @@
 		"re-resizable": "4.7.1",
 		"react": "16.4.1",
 		"react-dom": "16.4.1",
+		"react-sortable-hoc": "0.8.3",


How many bytes does this add to our bundle size?

Around 40 KB

caxco93 · 2018-08-03T07:05:46Z

Thanks for spotting that. It should be addressed now.

caxco93 · 2018-08-03T07:15:46Z

This react-sortable-hoc is raising some React warnings though. And I am also starting to notice some performance issues. Trying to decide if we should keep using it or maybe roll our own. I originally intended to use the HTML5 Drag and Drop API but had problems with the GalleryImages being nested inside another draggable.

ehg · 2018-08-03T12:00:45Z

Trying to decide if we should keep using it or maybe roll our own.

What are the trade-offs between using it or rolling our own?

ehg · 2018-08-03T12:06:25Z

There are some code formatting/style issues that should be fixed. Are you running a linter?

ehg · 2018-08-03T12:07:14Z

core-blocks/gallery/sortable.js

+const SortableItem = SortableElement( ( { children } ) => children );
+
+class Sortable extends Component {
+	//constructor


Why comment here?

Also, we could get rid of the constructor all together using by using class properties.

You are right let me fix that

we don't have support for class properties enabled though, and other blocks/components seem to be relying on binding inside the constructor as well. Should we enable it?

We're reluctant to use new language features until they've reached stage 4. Class fields are still at stage 3.

caxco93 · 2018-08-03T17:12:30Z

Yes I am using eslint on vscode. You mean the function comments right? Thanks for noticing that. I'll check and solve it

caxco93 · 2018-08-07T04:48:21Z

@ehg Rolling our own one or a fork would let us modify the component that is being rendered while dragging and also address the performance issues directly; however, it would take more time.
Those are the two main missing points by now.

caxco93 · 2018-08-28T07:42:50Z

Hey, so I addressed the sizing issue when reordering. It now looks nice like this:

I was thinking I would need to set a class on the rearranging elements until I realized the dynamic size of the gallery items is not caused by a different class, but by their flex-grow property and their DOM index position inside the flex-wrappable parent.
The way I did it is by calculating the correspondent scale factor given the ratio of the different item dimensions and setting it as its transform: scale 👍

However, this behaviour has been achieved by modifying the dependency library react-sortable-hoc locally.

I created a PR there (clauderic/react-sortable-hoc#422), but he doesn't really look that active right now. 👎

oandregal · 2018-08-30T09:10:49Z

Hey, just wanted to note that if it crossed your mind to get rid of react-sortable-hoc and implement this behavior within Gutenberg, you may take advantage of the Draggable component. It's being refactored at #9311 to enabling its use in use cases other than drag blocks around (for example, dragging pictures!) - it's almost ready to be merged.

ghost · 2018-08-30T18:48:29Z

@nosolosw Can you just provide us with full example for how to use the Draggable component in some use cases other than drag blocks.

oandregal · 2018-08-30T19:42:47Z

@muhammedmagdi the PR I've mentioned also updates the docs, see README for an example. See also the BlockDraggrable component for a concrete implementation, it makes use of Draggable.

That PR is now approved and it's going to be merged into master next week, after the 3.7 Gutenberg release. From that point on, it'll be available for anyone to use.

ghost · 2018-08-30T19:46:33Z

@nosolosw
Thank you.

* Created new Sortable HOC for wrapping the Gallery Images * Added react-sortable-hoc dependency * Right now you have to hold down to reorder

Altered the Sortable component so it now handles custom first and last nodes.

Addresses the floating square when dragging

Got rid of closure higher order component in favor of readability

The Gallery Item Image did not have proper styling because of the way the dragged element is created (outside its <ul>) This adds the necessary css to address this.

gziolo · 2019-02-01T08:50:17Z

@mapk and @youknowriad - what's the plan regarding this feature for Phase 2?

youknowriad · 2019-02-01T08:58:59Z

@gziolo It just needs dev :)

oandregal · 2019-02-01T09:47:57Z

@gziolo @youknowriad I had this on my radar to work on after my the doc generator task. We may want to close this PR and reference the master issue #743 everywhere (it contains other references). I'll just do that. If we end up using this approach, we can always reopen.

gziolo · 2019-02-01T09:50:17Z

Awesome, thanks @nosolosw for making it clear. Indeed, we can always reopen as we get back to this issue. Let's keep the number of open PRs in a healthy state :)

Soean added [Feature] Media Anything that impacts the experience of managing media [Feature] Drag and Drop Drag and drop functionality when working with blocks labels Jul 13, 2018

caxco93 force-pushed the update/gallery-drag-and-drop-reorder branch 2 times, most recently from 6b893d1 to 19a7dc8 Compare July 25, 2018 03:56

caxco93 force-pushed the update/gallery-drag-and-drop-reorder branch 2 times, most recently from 91c3c91 to f1b4e55 Compare August 1, 2018 00:00

noisysocks reviewed Aug 1, 2018

View reviewed changes

ehg reviewed Aug 3, 2018

View reviewed changes

caxco93 force-pushed the update/gallery-drag-and-drop-reorder branch from 201b54e to ba90922 Compare August 6, 2018 00:48

caxco93 added 2 commits September 3, 2018 22:54

Implemented Gallery Drag and Drop Reordering

9fa1c5b

* Created new Sortable HOC for wrapping the Gallery Images * Added react-sortable-hoc dependency * Right now you have to hold down to reorder

Properly extracted prop function

bef6f2e

caxco93 added 9 commits September 3, 2018 22:56

Fixed the missing "Upload an image"

507ac41

Altered the Sortable component so it now handles custom first and last nodes.

Changed drag behaviour so it 'just works'

8163529

Added missing class to the dragged element

8480d5e

Addresses the floating square when dragging

Added shouldComponentUpdate to prevent unncessary renders

d9fc48d

Modified imports to use proper WordPress packages

bd3bc79

Removed unnecessary prop mutation that raised a warning

c9e4453

Extracted Sortable HOC

3d7d861

Got rid of closure higher order component in favor of readability

Fixed minor code formatting/style issues

8e6167f

Corrected misbehaving selection after drop reorder

16e537e

caxco93 force-pushed the update/gallery-drag-and-drop-reorder branch from 3ab09fb to 5fef65b Compare September 4, 2018 06:05

Fixed styling issue on currently dragged element

e1357bf

The Gallery Item Image did not have proper styling because of the way the dragged element is created (outside its <ul>) This adds the necessary css to address this.

caxco93 force-pushed the update/gallery-drag-and-drop-reorder branch from 5fef65b to e1357bf Compare September 4, 2018 06:31

caxco93 mentioned this pull request Sep 17, 2018

Gallery Drag and Drop Reordering [Using Draggable alternative] #9945

Closed

4 tasks

Soean added the [Block] Gallery Affects the Gallery Block - used to display groups of images label Oct 26, 2018

oandregal closed this Feb 1, 2019

simison mentioned this pull request Apr 3, 2019

Tiled gallery block: support re-ordering images by dragging Automattic/jetpack#11874

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented Gallery Drag and Drop Reordering #7946

Implemented Gallery Drag and Drop Reordering #7946

caxco93 commented Jul 13, 2018 •

edited

Loading

ajitbohra commented Jul 13, 2018

caxco93 commented Jul 17, 2018

caxco93 commented Jul 17, 2018

karmatosed commented Jul 18, 2018

ZebulanStanphill commented Jul 18, 2018

caxco93 commented Jul 19, 2018 •

edited

Loading

caxco93 commented Jul 30, 2018 •

edited

Loading

noisysocks commented Aug 1, 2018

noisysocks Aug 1, 2018

caxco93 Aug 3, 2018

caxco93 commented Aug 3, 2018

caxco93 commented Aug 3, 2018

ehg commented Aug 3, 2018 •

edited

Loading

ehg commented Aug 3, 2018

ehg Aug 3, 2018

ehg Aug 3, 2018

caxco93 Aug 3, 2018

caxco93 Aug 6, 2018

noisysocks Aug 6, 2018

caxco93 commented Aug 3, 2018

caxco93 commented Aug 7, 2018

caxco93 commented Aug 28, 2018 •

edited

Loading

oandregal commented Aug 30, 2018

ghost commented Aug 30, 2018

oandregal commented Aug 30, 2018

ghost commented Aug 30, 2018

gziolo commented Feb 1, 2019

youknowriad commented Feb 1, 2019

oandregal commented Feb 1, 2019

gziolo commented Feb 1, 2019

Implemented Gallery Drag and Drop Reordering #7946

Implemented Gallery Drag and Drop Reordering #7946

Conversation

caxco93 commented Jul 13, 2018 • edited Loading

Description

Screenshots

Types of changes

Checklist:

KNOWN ISSUES:

ajitbohra commented Jul 13, 2018

caxco93 commented Jul 17, 2018

caxco93 commented Jul 17, 2018

karmatosed commented Jul 18, 2018

ZebulanStanphill commented Jul 18, 2018

caxco93 commented Jul 19, 2018 • edited Loading

caxco93 commented Jul 30, 2018 • edited Loading

noisysocks commented Aug 1, 2018

noisysocks Aug 1, 2018

Choose a reason for hiding this comment

caxco93 Aug 3, 2018

Choose a reason for hiding this comment

caxco93 commented Aug 3, 2018

caxco93 commented Aug 3, 2018

ehg commented Aug 3, 2018 • edited Loading

ehg commented Aug 3, 2018

ehg Aug 3, 2018

Choose a reason for hiding this comment

ehg Aug 3, 2018

Choose a reason for hiding this comment

caxco93 Aug 3, 2018

Choose a reason for hiding this comment

caxco93 Aug 6, 2018

Choose a reason for hiding this comment

noisysocks Aug 6, 2018

Choose a reason for hiding this comment

caxco93 commented Aug 3, 2018

caxco93 commented Aug 7, 2018

caxco93 commented Aug 28, 2018 • edited Loading

oandregal commented Aug 30, 2018

ghost commented Aug 30, 2018

oandregal commented Aug 30, 2018

ghost commented Aug 30, 2018

gziolo commented Feb 1, 2019

youknowriad commented Feb 1, 2019

oandregal commented Feb 1, 2019

gziolo commented Feb 1, 2019

caxco93 commented Jul 13, 2018 •

edited

Loading

caxco93 commented Jul 19, 2018 •

edited

Loading

caxco93 commented Jul 30, 2018 •

edited

Loading

ehg commented Aug 3, 2018 •

edited

Loading

caxco93 commented Aug 28, 2018 •

edited

Loading