nick-keller / react-datasheet-grid

An Airtable-like / Excel-like component to create beautiful spreadsheets.
MIT License
1.81k stars 170 forks source link

Component going to infinite renders, if I update state in On Blur #230

Open avp1598 opened 1 year ago

avp1598 commented 1 year ago

On Blur event keeps getting triggered infinitely if some state is changed in on blur callback I have checked all useEffects and everything, the problem is with the component itself. Just the onBlur event being called infintely and for any type of cell update.

I am using a dynamicDatasheet as I need the columns to be dynamic

nick-keller commented 1 year ago

Thx for the report @avp1598, Can you provide a sandbox to demonstrate the issue?

kivircik-parantez commented 1 year ago

On Blur event keeps getting triggered infinitely if some state is changed in on blur callback I have checked all useEffects and everything, the problem is with the component itself. Just the onBlur event being called infintely and for any type of cell update.

I am using a dynamicDatasheet as I need the columns to be dynamic


const [selectionState, setSelectionsState] = useState<SelectionWithId | null>(null);  
const selectionCallback = useCallback(
    (opt: { selection: SelectionWithId | null }) => {
      if (!_.isEqual(opt.selection, selectionState)) {
        setSelectionsState(opt.selection ?? selectionState);
      }
    },
    [selectionState]
  );```

I solved the problem by giving onSelectionChange to selectionCallback . If it helps. @avp1598 
larissa-n commented 1 year ago

https://codesandbox.io/s/fervent-khayyam-oqzfps?file=/src/App.js

@nick-keller you'll notice: AFTER the table was focused & blurred at least once, the onBlur function is fired EVERY TIME you add a row or "generate table data", i.e. every time you update table state externally.

This seems like a bug.

larissa-n commented 1 year ago

https://github.com/nick-keller/react-datasheet-grid/blob/92fdf7d0e01e4ccdd1385423efc4abf7b714eaf9/src/components/DataSheetGrid.tsx#L1728

Is the lastEditingCellRef reset after an onBlur event?

larissa-n commented 1 year ago

By the way, I think it would be neat if the onBlur event had a newValue argument like onChange does. Would make it more robust in cases where onBlur is used to get the new state, like if getting the new table state with every keystroke is performance overkill. As it is, you'd have to sync from the spreadsheet controlled state variable onBlur, which is a problem if onBlur fires every time you sync to the table controlled state. Thanks!

Also: Pasting into the datasheet (incl. clicking in and out) does not trigger an onBlur event, right? Should it?

larissa-n commented 1 year ago

onSelectionChange is triggered every time anything about the spreadsheet content changes, the spreadsheet first renders, re-renders, an undo-function changes its state, and on every single keystroke when typing in a cell. Not sure if that's as intended, as the Docs say it should only be called when the selection changes or the grid is blurred.

Sandbox: https://codesandbox.io/s/cool-platform-8083xp?file=/src/App.js

larissa-n commented 1 year ago

@nick-keller any updates on this yet? Thank you!