DataTable: Tabulation is not working propertly. Also editorCallback does not reflect the new value in some cases.

[didix16]

Describe the bug

Hello there.

After updating to the last version 10.6.6, I'm facing new issues with DataTable... Now tabbing between cells seems broken. What I'm experimeting is that now, changing to next cell is not inmediate, I mean, current cell lost focus but on tab, the next cell is not opened nor focused (I don't know where the tab focus goes). You have to enter Tab again to change to the next cell...

You can experiment the behaviour in the reproducer.

Also going backwards (Shift + Tab) It seems don't work. It stucks on the cell. However if you keep pressing Shift + Tab, eventually it jumps backwards.

And finally, if I go to a cell where I have an Autocomplete component, I select a value from it then when I press Tab does nothing. It does not change to next cell.

This is because when executing cellEditValidator, the event is returning a value which is not the desired value. If I type "a" on autocomplete component on a cell table, it updates the the row value to that value. But if I select the autocomplete value using the keyboard (arrows + enter), which the resulting value is an object, the row value seems not to be updated. However the editorCallback is executed but the internal rowData is not updated. @kl-nevermore I think your change in #6641 needs to be fixed for that case, when updating the rowData using the editorCallback.

What I've noticed is that ...editingRowDataStateRef.current on switchCellToViewMode does not contains the updated value from editorCallback. So The useEffect() is not being executed after calling setEditingRowDataState(editingRowData); on editorCallback and thus, when switchToCellViewMode ends, sets the same old value that row has, avoiding the new value.

Hope the errors are understanded.

Reproducer

https://stackblitz.com/edit/vitejs-vite-9p4ifk?file=src%2FExample.tsx

PrimeReact version

10.6.6

React version

18.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

Any

Steps to reproduce the behavior

To reproduce the TAB error.

1. Go to a editable cell on table
2. Press TAB. See that the next cell is not being selected
3. Press TAB again. Next cell is selected
4. Try to go backward using Shift + TAB. If the cell is not an autocomplete, the cell closes and does nothing else.
5. Press again shift + TAB on the same cell. The same cell is focused again, instead of previous.

To reproduce the Autocomplete error:

1. Go to a cell with autocomplete edior.
2. Type "a" then press TAB.
3. The autocomplete value has "Agent007" but the value returned in validator is false and thus the cell is not closing.

You can see the console.logs to track the value changes.

Expected behavior

For the TAB.

That works like before, on TAB, change to next cell inmediatly and also Shift + TAB returns to previous cell and inmediatly.

[didix16]

I've tried to rollback before #6641 modification and seems to work fine. So it's related on how switchCellToViewMode handles now the changes and maybe the useEffect for refresh the cache row state.

[didix16]

After doing some tests, I've found something. It seems that now event.preventDefault() MUST NOT be called inside onCellEditComplete. After removing this, with the current 10.6.6 version, the tabulation works ALMOST as expected. The only problem I found is that Shift + TAB can't change from first cell table to any other focusable element.

@melloware I think the documentation examples should be updated, removing the event.preventDefault() on onCellEditComplete examples.

Also I'm gonna make a PR fixing the editorCallback to properly update the value.

So finally it remains that strange bug that Shift + TAB can't change from first cell table to any other focusable element

[melloware]

Agreed. Did you want to submit a PR?

[melloware]

Nevermind see you did!

[melloware]

If you want to fix both issues in your PR I will review.

[didix16]

I would like to fix also the bug when you press Shift + TAB on first cell but at the moment I don't how to fix it without rolling back the old code. I just only found a solution for what I mentioned before. Hope someone could help

[melloware]

@kl-nevermore can you read this ticket please since the change in question was your recent change?

[kl-nevermore]

@didix16
I removed useEffect and manually sync editingRowDataStateRef according to the original plan
and removed event.preventDefault() in onCellEditComplete
The gif should be the expected behavior, right?

[kl-nevermore]

I think it should be the order of execution timing of useEffect and callback.

[didix16]

@didix16
I removed useEffect and manually sync editingRowDataStateRef according to the original plan
and removed event.preventDefault() in onCellEditComplete
The gif should be the expected behavior, right?

@kl-nevermore yeah that should be the expected behavior indeed. By the way, what about returning from 1rst cell to another component, using shift + TAB?

[kl-nevermore]

@didix16 @melloware
I'm not sure which one is correct

changed

10.6.5

[didix16]

Mmm maybe the changed version has more sense in terms of accessibility. It allows to focus on column headers and interact with them.

[melloware]

I would assume it accessibility related to navigate to headers.

[kl-nevermore]

@melloware
Do you know why setTimeout is used here?

const focusOnElement = () => {
        clearTimeout(tabindexTimeout.current);
        tabindexTimeout.current = setTimeout(() => {
           
            if (editingState) {
                const focusableEl = props.editMode === 'cell' ? DomHandler.getFirstFocusableElement(elementRef.current, ':not([data-pc-section="editorkeyhelperlabel"])') : DomHandler.findSingle(elementRef.current, '[data-p-row-editor-save="true"]');

                focusableEl && focusableEl.focus();
            }

            keyHelperRef.current && (keyHelperRef.current.tabIndex = editingState ? -1 : 0);
        }, 1);
    };

[kl-nevermore]

@didix16
submitted a PR, can you test it?
I tested it and looks ok

[didix16]

Hey @kl-nevermore. I've tested it and yeah seems that works as expected.

[melloware]

merged!