react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

TableView rows don't re-render if values change other than the key

Open rabidpug opened this issue 3 years ago • 11 comments

🐛 Bug Report

TableView rows don't re-render when values change if the key does not change.

🤔 Expected Behavior

Rows should re-render if their values change

😯 Current Behavior

Rows don't re-render if their values change

💁 Possible Solution

Should be able to opt out of memoization.

🔦 Context

I have a table where the user selects rows (which by design are identified by their Key, so I have to use the row ID), then clicks a button to approve/reject rows. The rows don't re-render with the changed values.

💻 Code Sample

CodeSandBox demonstrating

🌍 Your Environment

Software Version(s)
react-spectrum 3.11.2 / 3.0.0-beta.1 (for table)
Browser Chrome 90
Operating System Windows 10

rabidpug avatar Jul 05 '21 00:07 rabidpug

I think the rows actually do re-render, but looks like the cells are not invalidated.

devongovett avatar Jul 05 '21 01:07 devongovett

In collection builder we are not rerendering the cells when the row is updating. We might need to force rerender all cells when the value(s) in the row changes.

dannify avatar Jul 21 '21 17:07 dannify

@rabidpug Looks like this is actually related to StrictMode, removing that makes the TableView rows rerender properly when typing in input field. Definitely something we need to address as a whole w/ our library

LFDanLu avatar Jul 27 '21 20:07 LFDanLu

I was experiencing this as well and removing StrictMode didn't change the behavior.

JSanchezIO avatar Nov 24 '21 01:11 JSanchezIO

@JSanchezIO Mind sharing a codesandbox of the issue?

LFDanLu avatar Nov 29 '21 21:11 LFDanLu

I'm now re-reading the title of this and realize that my issue isn't with <TableView /> but with useTable and its supporting hooks. I can still try to work on a repro if you'd like.

JSanchezIO avatar Nov 30 '21 20:11 JSanchezIO

In that case feel free to open a separate issue/discussion and we can help you there.

LFDanLu avatar Nov 30 '21 20:11 LFDanLu

Was brought up in slack room with https://codesandbox.io/s/tableviewv3-sort-forked-eppm1?file=/src/App.js (slightly modified from the original) as an example, looks like it isn't just a strict mode issue.

LFDanLu avatar Dec 09 '21 19:12 LFDanLu

It looks like this is more general than TableView and it was found in ListView. See the https://codesandbox.io/s/listview-item-height-test-forked-w3iwcn. Clicking on an item changes its value and moves it to the end of the list where the item displays the update. If we only update the item, the UI doesn't change.

ktabors avatar Jul 14 '22 19:07 ktabors

A workaround was found. If you move the TableHeader to be after the TableBody in the TableView it works correctly. This modifies the example Daniel shared above to show this workaround. https://stackblitz.com/edit/stackblitz-starters-xmx5ag

ktabors avatar Feb 14 '24 20:02 ktabors

Why does this bug still exist? @ktabors solution worked for me but only after I pulled my hair out for four hours.

Edit: I'm trying to recreate my issue in a simplified form in CodeSandbox, but I can't seem to do it.

frankieali avatar Apr 18 '24 19:04 frankieali