Skip to content

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
@dkamburov dkamburov dismissed damyanpetev’s stale review August 12, 2025 07:40

comments applied

@dkamburov dkamburov merged commit aa1840d into vnext Aug 12, 2025
2 checks passed
@dkamburov dkamburov deleted the ttonev/excel-style-editing-new branch August 12, 2025 07:40
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.

7 participants