feat(table): Support drag and drop of rows within the table

[gillibrand]

Closes #3751

Summary

Adds a new feature that allows table rows to be dragged around and dropped into other rows of the same table. This is designed to use with tables that use "file" and "folders" style data, where a file row should be able to be dropped onto a folder row. The IBM ELM product suite is driver for this request.

The storybook doc (Table > DragAndDrop.mdx) has details on the properties required to opt in to this feature.

The visual design during a drag comes from the recommendations at https://pages.github.ibm.com/cdai-design/pal/patterns/drag-and-drop/usage/. Adding the dashed border around the row is tricky since it can't be done with CSS around the row/cells. Instead, or create a new abs positioned element just for the border and move it around over the rows (TableDropRowOverlay.jsx).

There is a new leading cell to hold the drag handle on draggable rows. This is done like the checkboxes for selectable rows: its in its own tell with an empty <th> in its column.

There is no keyboard support or accessibility labels for the drag handle. This feature is considered supplemental. There are expected to be be "Move" or "Cut/Paste" row menu actions to perform similar operations with the keyboard.

Change List (commits, features, bugs, etc)

There are a number of small changes in the Table are and some small supporting components, but the majority of the work is is in useTableDnd.jsx. This is a hook only to extract that code into its own file--it's not expected to be reusable in other places. The associated Issue describes some reason why a custom implementation was chosen. That file looks big, but it's probably half comments.

mousedown on a drag handle triggers things to start which causes isPossibleDrag to be set to true. That fires useEffect to attach listeners for mouse events. If the mouse moves past a threshold (5px) then it's considered a real drag (not a rogue click) and isDragging is set to true. At that point an avatar (drag image) is created next to the cursor--its content is returned from the onDrag callback prop on the table. The mousemove listener updates the avatar to follow the cursor. To know what row we're over mouseenter and mouseleave events are then added to all the rows that can be dropped onto (also returned from the onDrag callback).

The user can cancel the drag with a mouseup over anything that's not a drop row, or explicitly with the Esc key. If they don't cancel, releasing over a drop row fires the onDrop callback and resets all drag state and unregisters listeners (except for mousedown on the drag handles). It's up to the consumer to decide how to handle the drop. The storybook has a trivial example that simulates moving rows into other rows.

Acceptance Test (how to verify the PR)

• Storybook: Table > with drag and drop rows. File rows should be draggable onto folder rows. Dragging a selected row includes all the selected rows in the drag. (Note that the drag preview is supplied by the caller, the story here, so ignore its ugly padding.)
• New test that are part of StatefulTable.test.jsx.

Regression Test (how to make sure this PR doesn't break old functionality)

• Existing table tests.

Things to look for during review

• [ ] Make sure all references to iot or bx class prefix is using the prefix variable
• [ ] (React) All major areas have a data-testid attribute. New test ids should have test written to ensure they are not changed or removed.
• [ ] UI should be checked in RTL mode to ensure the proper handling of layout and text.
• [ ] All strings should be translatable.
• [ ] The code should pass a11y scans (The storybook a11y knob should show no violations). New components should have a11y test written.
• [ ] Unit test should be written and should have a coverage of 90% or higher in all areas.
• [ ] All components should be passing visual regression test. For new styles or components either a visual regression test should be written for all permutations or the base image updated.
• [ ] Changes or new components should either write new or update existing documentation.
• [ ] PR should link and close out an existing issue

[netlify[bot]]

✅ Deploy Preview for carbon-addons-iot-react ready!

Name	Link
🔨 Latest commit	b6e7f030de64acd0e04fcc474c2d0822cb19382a
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-addons-iot-react/deploys/64260da3a270b500089bee5f
😎 Deploy Preview	https://deploy-preview-3756--carbon-addons-iot-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[gillibrand]

@davidicus I just tested this on Mac with a Magic Mouse and found that it's very hard to drop rows. The drop is aborted most of the time, and I don't know why yet. I'm guessing a Mac trackpad is the same. A "normal" mouse on Mac works fine. I'll debug it tomorrow. Ugh.

[gillibrand]

@davidicus Fixed the macOS problem. Data could be set out of order sometimes and the drop wouldn't have access to the hovered row. Not sure why only a Magic Mouse triggered it, but works reliably now.

[gillibrand]

Added a commit to fix RTL layout. Now works well in RTL.

[davidicus]

Hey @gillibrand Looking at this now. Is it possible to add this functionality to our playground story. Interested in testing this feature in combination with other features. Are there features that we know are mutually exclusive with this one?

[JordanWSmith15]

This is a great first pass. My desire sort of relates to @davidicus comment above, in that, what if some other table feature that impacts the rows is active? It would be nice to see this property behaving in those scenarios. The main ones that impacts the rows that we should prove out are:

• Nesting
• Multi-select/radio single select

It may not be necessary for your use case, but I think we need an example of these, or a clear restriction saying that they do not work. For nesting, it would be nice to see that a user can drag items back out of the parent, rather than sort of "cheating" it with that reset button you have in your example.

[gillibrand]

Multi-select is OK, and is demoed in my first story.

Row expansion has problems though. There are also some hover states seen in playground that don't make sense during a drag. I'll look into both of those.

[gillibrand]

@davidicus @JordanWSmith15 I've updated the Table > Playground to support DnD. For example, you can drag child rows between different parents. There are some limitations:

• You can only drag to rows, so there's no way to make a child a top level row. With the right data there could always be a top-level "root" row that can never be dragged, like a tree. Otherwise, there would need to be a new drop target to support dropping into the "root" data. Since I don't have this case don't plan to address it.
• Multi-select is a little weird with children. If you select all child rows, the parent row is implicitly selected. So moving all children always selects and moves the parent too. This seems inherent to the table design, but I'm not sure why. Seems like any batch action would have this limitation too (e.g., deleting all children). If there's a prop to change that behavior, I missed it.

[gillibrand]

Drag and Drop is a new knobs group to enable and disable.

[gillibrand]

@JordanWSmith15 Turned out the class to hide the expando was only ever added to closed rows (probably because there was no way to open a row without children before). I added the same conditional class for open state rows and now the exando will hide as soon as all children are removed. Just pushed that commit.

[JordanWSmith15]

@gillibrand @davidicus

We need to verify that we do not need to deliver an a11y version for this since we have a huge a11y push this sprint. @JordanWSmith15 thoughts on a11y and our need to deliver along with this?

For accessibility, I'd say that once this gets into Graphite, we need to check the AVT1 scans to see if this is causing any new violations. Those would need to get fixed in PAL, which means it may be worth your time to just add the IBM Scan tool to PAL as well at some point. Assuming AVT1 scans are clean, I think we are good in terms of the component itself... but, for AVT2/3 compliance inside of the application itself, there is a lot of nuance. In general terms, there needs to be a method for the user to achieve the goal (anyhow, anywhere) in the product using the keyboard. For example, you could add buttons that move the rows up and down one at a time and retain focus on the current row. Another option would be not moving the rows themselves up and down using the table itself; it could be done on a different screen/modal with a different approach. It could even be done in a totally different app, like your "legacy" application (if the rest of it is accessible), and still be considered a pass.

[gillibrand]

@JordanWSmith15 For the ELM case (Report Builder) we have a row action to "Move" items already. That and the dialog it opens can be navigated with the keyboard. Drag and drop is considered an alternative way to do a move. So the DnD feature is not really accessible, and will always needs a fallback.

Note that the visual grab handle I added the has role="presentation" to generally hide it from screen readers. It also does not have a tooltip or aria-label. Maybe it should, but that would need to be passed in from the app to know what the right string (plus I don't think PAL ships with localized messages anyway).

[gillibrand]

I've added a zIndex prop to the table, which is optional and defaults to 0. If passed, it doesn't affect the the table itself, only the absolutely positioned drag image and row overlays. They will actually use the given zIndex as a minimum and add the same +1000 and +1001 to it which they were doing in CSS before. I think that should satisfy the Graphite need to be able to pass a z-index when the table is used in a modal.

[gillibrand]

I also made a change to dramatically improve dragging performance. I noticed with a larger table, and especially with nested rows, dragging was sluggish. That's because it made a state change each time the hover row changed, which caused the whole table to rerender. I changed it to just set the "hovering" class on the row element directly with a DOM API instead of a React state update. The element was already available from from the mouseenter event.

[davidicus]

@JordanWSmith15 I was going to suggest that perhaps we add a dev warning when using this prop without the nested row prop but it looks like Jay's use case is one without hierarchies. I still do not see an affordance to undue. Since this will be consumed via Graphite is there anything else you think will be needed here?

[JordanWSmith15]

I guess it's a good question for @gillibrand and maybe the designer: If you drag an item into the wrong folder, how do you move it back out/into the correct folder?

[gillibrand]

In our current app UI a "move" operation shows a toast notification with an Undo button. That's not related to this DnD feature or the table per se, just how we happen to handle this case. So my answer is, it depends on the app. The table just has a callback that says where a row was dropped, but anything can happen in that case (though Move is the obvious thing to do). In the Storybook that callback moves the data client side. In a real app it's likely serve call with some type of table refresh.

[gillibrand]

Also, I took a look a Graphite on what needs to be done to adopt this. The only unusual thing is that the "onDrag" callback is synchronous with a return value, whereas most Graphite events are fire-and-forget. Sean already proposed a way to handle that though.

I assume what I added for z-index is OK, but I added that based only on what I understand from the comments here.

[gillibrand]

@davidicus From his Slack status, it seems Sean isn't around this week. Are you ok to merge anyway? Could I document the new props as "experimental" or something for now in case changes are needed?

[JordanWSmith15]

@gillibrand If Sean left comments and those have been resolved then I think we are OK, however, this feature won't do much for you without a backmerge of PAL in Graphite, which isn't happening for another sprint. We can probably hang tight.

[gillibrand]

Ok. That was exactly my motivation, to get this delivered and working on the Graphite bit. No need to hurry up and wait if that wont happen soon though.

[sls-ca]

@davidicus I approved this but it still says it cannot be merged. not sure why.

[jessieyan]

I think it still asks for @davidicus approval.