[RAC] App freezes when mounting disabled table rows after selection

[levrik]

Provide a general summary of the issue here

The app freezes completely when swapping table rows by disabled ones with different key.

🤔 Expected Behavior?

The app should not freeze.

😯 Current Behavior

The app freezes.

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

https://codesandbox.io/p/sandbox/fervent-architecture-6g96p6

1. Select 1 or more rows
   • You must not select the 1st row only
   • Ensure the blue focus outline appears on the row (it sometimes does not in Chromium-based browsers likely due to the iframe, native :focus has same issues)
2. Click "Toggle" button to swap out rows
3. App freezes

Remove isDisabled from all loading rows in code and try again. It does not freeze anymore.

React.Aria.Table.Row.Freeze.mp4

Version

1.5.0

What browsers are you seeing the problem on?

Firefox, Chrome, Safari, Microsoft Edge

If other, please specify.

No response

What operating system are you using?

macOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[LFDanLu]

Thanks for catching this, took a little bit but it seems like the root of this issue is with how we try to find a new row to focus if the previously focused row doesn't exist anymore. Since every row is disabled in the new set of items this portion of the code keeps looping. Off the top of my head, we can probably break out of it by simply flagging when it hits the else a second time but it might be worthwhile adding a function to SelectionManager/Collection that informs us if every item in the collection has been disabled via applying isDisabled to each Row/Item

[nwidynski]

@LFDanLu Just FYI on the proposed SelectionManager changes. While working on edit mode, I also recall running into focus issues due to all nodes being disabled. When entering, focus would be placed on the scrollRef even though it should arguably skip over the entire collection, especially when no loader is present. Leaving this here for reference:

react-spectrum/packages/@react-aria/table/src/useTable.ts

Line 92 in fa5ee06

isDisabled: props.isDisabled || state.collection.columns.every(c => state.selectionManager.isDisabled(c.key)),

[LFDanLu]

@nwidynski Thanks for the heads up! Off the top of my head, I think we would need to preserve a portion of scrollRef focusing behavior, at least for the case where the user was currently on a row and it gets deleted/disabled since otherwise focus would get lost to the body which is problematic for screenreader users. I also think the scrollRef should be tabbable from the outside if there is loader and/or an empty state message that would be beneficial for a screenreader user to access but it does feel like skipping over the collection when it is truly disabled makes sense since it would be similar to buttons/other disabled form elements.