Skip to content

Updated Excel edit style sample to functional component (React v19) #777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: vnext
Choose a base branch
from

Conversation

tishko0
Copy link
Contributor

@tishko0 tishko0 commented May 7, 2025

…o navigate correct

@tishko0 tishko0 changed the title fix(sample): updated sample to func component and fixed cell update t… fix(sample): updated excel edit style sample to functional component May 9, 2025
@MarielaTihova MarielaTihova changed the title fix(sample): updated excel edit style sample to functional component Updated Excel edit style sample to functional component (React v19) May 9, 2025
@damyanpetev damyanpetev changed the base branch from vnext to v19-updates May 14, 2025 10:53
@damyanpetev damyanpetev added the squash-merge Please squash the commits in this PR label May 14, 2025
Comment on lines 41 to 86
gridRef.current.markForCheck();
} else

if (activeElem && activeElem.editMode && shouldAppendValue.current) {
event.preventDefault();
activeElem.editValue = activeElem.editValue + event.key;
shouldAppendValue.current = false;
}
}

if ((key >= 48 && key <= 57) || (key >= 65 && key <= 90) || (key >= 97 && key <= 122)) {
var columnName = grid.getColumnByVisibleIndex(activeElem.column).field;
var cell = grid.getCellByColumn(activeElem.row, columnName);
if (code === 'Backspace') {
if(activeElem == null) {
return;
}
const rowIndex = activeElem.row.index;
const columnKey = activeElem.column.field;

gridRef.current.data[rowIndex][columnKey] = '';
gridRef.current.markForCheck();

if (cell && !grid.crudService.cellInEditMode) {
grid.crudService.enterEditMode(cell);
cell.editValue = key;
}
}

if (key == 13) {
var thisRow = activeElem.row;
var column = activeElem.column;
var rowInfo = grid.dataView;
if (code === 'Enter' || code === 'NumpadEnter') {

if(activeElem == null) {
return;
}

const thisRow = activeElem.row.index;
const dataView = gridRef.current.dataView;
const nextRowIndex = getNextEditableRowIndex(thisRow, dataView, event.shiftKey);

gridRef.current.navigateTo(nextRowIndex, activeElem.column.visibleIndex, (obj: any) => {

requestAnimationFrame(() => {
gridRef.current.endEdit(true, obj);
gridRef.current.clearCellSelection();
obj.target.activate();

});
});

var nextRow = this.getNextEditableRowIndex(thisRow, rowInfo, (args.detail.event as any).shiftKey);
}
};

grid.navigateTo(nextRow, column, (obj: any) => {
obj.target.activate();
grid.clearCellSelection();
});
}
}
gridElement.addEventListener("keydown", handleKeyDown);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple points:

  • Why is this stuck in useEffect? The old sample used gridKeydown which should now be onGridKeydown and should be used instead
  • What's with the gridRef.current.markForCheck();, requestAnimationFrame and endEdit calls? Those aren't needed in the original source (ng) and shouldn't be here as well. If they are, we might want to investigate why. Especially the rAF since it's in the already delayed navigateTo callback

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik onGridKeyDown does not trigger when using any key, just the ones that are in navigation and have grid functions. Have this changed in 19? Initially I had it with onGridkeyDown but I remember having issues with this.
For the second one, the RAF is needed as the navigate is async, so I get timing issues and the cell does not exit edit mode in time, so this was the solution I came up with. Open to discussion to improve it as it does not feel very smooth

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh cool, just noticed that in https://www.infragistics.com/products/ignite-ui-angular/angular/components/grid/cell-editing#angular-grid-excel-style-editing-sample
Great, we made a sample that modifies navigation without our own dedicated event for that. Cool, cool 😶

Fine onKeyDown then, at the very least still remove the useEffect and use the default way to attach a handler:
image
That works just fine and is equivalent w/ less code. Also note, the correct arg type here is React.KeyboardEvent<IgrGrid> instead of the lib.dom one and that means your grid ref is also:

const grid = event.currentTarget;

And that's both typed correctly and can replace the ref entirely. Same goes for the gridEndEdit, though you'll need to assert here. I'll leave a note there too and you can remove the ref entirely after.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assigned keydown and added handler outside of useEffect

Base automatically changed from v19-updates to vnext May 16, 2025 17:03
Comment on lines 24 to 25
var code = event.code;
var activeElem = gridRef.current.selectedCells[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also const instead of var please :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment on lines 104 to 106
function gridEndEdit(event: IgrActiveNodeChangeEventArgs): void {
gridRef.current.endEdit(true, event.detail);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note endEdit is not correctly called with event.detail, the second arg is for actual event and errors out. Also, no real public use should need it, so just replicate the original

Suggested change
function gridEndEdit(event: IgrActiveNodeChangeEventArgs): void {
gridRef.current.endEdit(true, event.detail);
}
function gridEndEdit(event: IgrActiveNodeChangeEventArgs): void {
const grid = event.currentTarget as IgrGrid;
grid.clearCellSelection();
grid.endEdit();
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, I see an error with the endEdit, so there might be an issue to log with the product bcuz...
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really sure how to proceed with this one, advise please

@MayaKirova MayaKirova added the status: in-test PR ready for testing label Jun 10, 2025
@MayaKirova
Copy link
Contributor

Only small issue I noticed is that Shift+Enter select all cells it moves through:
image

Probably selection needs to be cleared like in the angular sample.

@IMinchev64 IMinchev64 added status: verified The PR is tested and ready for a merge and removed status: in-test PR ready for testing labels Jul 4, 2025
@IMinchev64 IMinchev64 self-assigned this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash-merge Please squash the commits in this PR status: verified The PR is tested and ready for a merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants