silevis / reactgrid

Add spreadsheet-like behavior to your React app
https://reactgrid.com
MIT License
1.19k stars 131 forks source link

Number Cell does not support input in an Intl format #397

Open jsilvao opened 4 months ago

jsilvao commented 4 months ago

Describe the bug In Number Cells which have a Intl format, although the output is properly formatted and displayed in the cell, you cannot type in the cell as if you would be typing directly in the Intl format specified.

Current behavior For example, if I set the Number Cells to have Intl.NumberFormat('de-DE') as format which uses , as the decimal separator, I cannot type 1,34 as the comma would be dropped, I need to type 1.34 in order to see 1,34 in the cell.

Expected behavior It should be possible to input numbers in the same way as they are represented by the Intl format

Your environment details

webloopbox commented 4 months ago

@jsilvao I have tested the de-DE Intl format with the browser's preferred language set to German, and it appears to function correctly. Could you provide your code implementation?

https://github.com/silevis/reactgrid/assets/60100557/b6139457-5b99-42a6-a93c-36a65f6e3381

jsilvao commented 4 months ago

Hi @webloopbox . It's very simple, I'm just doing

cells: [
        {
          type: 'text',
          text:
            momentGranularity === GranularityEnum.ONE_WEEK
              ? `KW ${format(referenceMoment.toDate(), 'w yyyy')}`
              : formatDateByGranularity(referenceMoment.toDate(), momentGranularity),
        },
        { type: 'number', value: rowValues[i], format: new Intl.NumberFormat('de-DE', { maximumFractionDigits: 4 }) },
      ],

It's a 2 column grid, first column is prefilled with some dates, the second column are NumberCells. My browser is in English, but my colleagues' browser are in German, they still cannot type a , as it would be dropped, they need to type . for decimal separator, and then it would be displayed as ,.

I came with a hack, I wanted to capture onKeyPress events, check for a , and then send a KeyboardEvent as having pressed the .. However, there are no onKey... props exposed.

jsilvao commented 4 months ago

Hi @webloopbox , we managed to narrow down how to reproduce the issue. So, if you are in a Google Spreadsheet, and the cells are using non-German format, copying and pasting into the grid will drop the ,.

webloopbox commented 4 months ago

@jsilvao In ReactGrid, when the value of a NumberCell is changed, the parseLocaleNumber function is invoked. This function converts a string with a comma to a . The resulting value is then passed as a number to the Intl format function.

https://github.com/silevis/reactgrid/blob/cb86f7da7f7b36ac2331881f8d550c6ba2f4565c/src/lib/Functions/parseLocaleNumber.ts#L17

I suspect that the issue might be related to the handler function that's passed to the onCellsChanged prop of the ReactGrid component. Could you please share this function, along with the example parameter value that includes the changed cell data?

jsilvao commented 4 months ago

Hi @webloopbox , here's the declaration of the grid:

 <ReactGrid
                    rows={rows}
                    columns={columns}
                    stickyTopRows={1}
                    enableFillHandle
                    enableFullWidthHeader
                    minColumnWidth={width / 2 || 100}
                    enableRangeSelection
                    onCellsChanged={handleCellChanges}
                    highlights={highlightedCells}
                  />

Now comes handleCellChanges and its hierarchy

const handleCellChanges = (changes: CellChange<NumberCell>[]) => {
    // If any timestamp cell was changed, then do nothing since they are not editable
    if (changes.some(change => change.columnId === 'timestamp')) return
    setRowValues(prevValues => applyChangesToRowValues(changes, prevValues))
  }
const applyChangesToRowValues = (changes: CellChange<NumberCell>[], prevValues: number[]): number[] => {
    const updated = applyNewValue(changes, prevValues)
    setCellChanges([...cellChanges.slice(0, cellChangesIndex + 1), changes])
    setCellChangesIndex(cellChangesIndex + 1)
    return updated
  }
const applyNewValue = (changes: CellChange<NumberCell>[], prevValues: number[], usePrevValue = false): number[] => {
    changes.forEach(change => {
      const valueIndex = change.rowId
      const cell = usePrevValue ? change.previousCell : change.newCell
      prevValues[valueIndex] = cell.value
    })
    return [...prevValues]
  }

And finally the three states involved

const [cellChangesIndex, setCellChangesIndex] = useState(() => -1)
const [cellChanges, setCellChanges] = useState<CellChange<NumberCell>[][]>(() => [])
const [rowValues, setRowValues] = useState<number[]>([])
jsilvao commented 4 months ago

Hi @MichaelMatejko and @webloopbox , let me know if you were able to reproduce it and if the code is useful. Thanks a lot! 👍

webloopbox commented 4 months ago

Hi @jsilvao, your ReactGrid declaration looks good. I've tested it locally, however, it did not allow me to reproduce the problem. The result is exactly the same as in the uploaded video. Remember that you can implement your own custom NumberCell - docs