[RAC] Not possible to use "useLongPress" with Table Row - event and labelling props get filtered out.

[lukpsaxo]

Provide a general summary of the issue here

I am trying to use the useLongPress hook with react-aria-components Row component.

The props provided by useLongPress get filtered by filterDOMProps and are not rendered.

🤔 Expected Behavior?

event handlers and labelling like what is provided by useLongPress should work with the Row.

😯 Current Behavior

props are filtered by filterDOMProps.

💁 Possible Solution

filterDOMProps could allow labelling props so the aria-describedby is let on to the TR.

for the event handling - a propNames set can be included which allows the events required by useLongPress - onKeyDown, onClick, onDragStart, onMouseDown, onPointerDown, onPointerEnter, onPointerLeave, onPointerUp.

Please let me know if this is ok and I will make a PR.

🔦 Context

We have a requirement to allow LongPress and Press on a table row to trigger an action.

🖥️ Steps to Reproduce

N/A (Maybe this should be a feature request?)

Version

3.38.1 / 1.7.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows

🧢 Your Company/Team

Saxo Bank

🕷 Tracking Issue

No response

[snowystinger]

To clarify, what would happen on a short press? is there selection?

Asking because we use long press already https://react-spectrum.adobe.com/react-aria/Table.html#row-actions and I want to make sure this request isn't asking to overload the behaviours we've defined.

[lukeapage]

The requirement we have is for a table that on mobile a long press opens a menu with options for that row. On desktop there is a button in the row, it just is difficult to fit it in when the screen is narrow.

We have a seperate more debatable requirement for short press to navigate to a detail view.

The table in question is not selectable (it has no checkboxes etc)

[snowystinger]

Thanks for the extra information, I'll bring it up with the team

[lukpsaxo]

@snowystinger did you get a chance to bring it up?

We are happy to do any development work to get this resolved.

Please let me know if I shouldn't prompt or 5 days is enough time to wait etc. We are building a big project using react-aria and its important to us we have a good relationship.

On that note too, if there is anything we can do/contribute that would help the team out to allow you more time to discuss improvements, please reach out.

[snowystinger]

Not a problem, always thank you for a bump reminder. Just a heads up, but we are moving slowly last week and this week because we're in a hack week. It's possible I won't have a chance until this Friday or next Friday. I haven't forgotten and it's at the top of the list of discussion topics :)

[lukpsaxo]

somewhat related but I could open a seperate issue - we would also like onScroll to be available as a prop for the TableBody as we currently need to use the ref and attach events to it in order to sync between the head scrolling in order to have sticky columns and sticky headers work together.

[snowystinger]

We had some scroll syncing at one point in our React Spectrum implementation. I forget when we started S2 implementation if we would have needed to add something for that or if we had a want to handle it already. Could you open a discussion or issue about it?

Also, it might be something that you could solve with our Virtualizer more easily, scroll syncing can get a bit janky due to the async nature of the scrolling.
See https://react-spectrum.adobe.com/react-aria/Virtualizer.html
And our sticky column implementation https://github.com/adobe/react-spectrum/blob/main/packages/%40react-stately/layout/src/TableLayout.ts#L34
it only does the selection column right now, but could probably be extended to more.

[lukpsaxo]

@snowystinger
We cannot use your virtualizer because of this - #7820
and we would be happy to discuss us contributing this - we have some potential performance issues with our current bolt-on implementation.

@toketoketoke implemented our sticky columns, I believe that unfortunately the sync scrolling was required to get sticky headers and sticky columns to work together.

[tokewf]

Hi!

Yes, so our setup is a combination of several factors:

1. Uses arbitrary ancestor for vertical scrolling
2. Column headers sticky relative to that scrolling ancestor
3. Table itself handles horizontal scrolling (rows can be wider than table parent)
4. Sticky columns (first/last) sticky relative to edges of table
5. Uses position: sticky in CSS to avoid DOM node measuring and position: fixed in an IntersectionObserver callback

For the column headers to stick to the scrolling ancestor, there can't be an intermediary element with non-visible overflow between the headers and the scroll ancestor. Thus, for the column headers to be sticky, the header itself must handle its horizontal scrolling, instead of the table. This means that the table body must also handle its horizontal scrolling. With this, we need to synchronize the scroll state of the table header and the table body.

Hope this paints a clearer picture of the setup.

[snowystinger]

Brought up this issue with the team today. We think it's a valid use case and we are potentially interested in supporting it in Table. There is an extra complexity to this, drag and drop, which is also possible to initiate with long press.

For the moment, it may be easiest to use patch-package to pass the onLongPress prop all the way down since the hook is already in use. Or you could use the ref to attach your listeners.

If you want to use a PR as the basis of the patch-package, we can try out the various different interaction combinations.

Some other thoughts we had were using onContextMenu. We don't have context menus yet, but may some day.