F2 in cell toggles edit mode

[zewa666]

Clear and concise description of the problem

Hey there. I cannot remember if we already talked about this but to be consistent with general UX behaviors, would hitting F2 inside a cell with dblclick to enter editor mode be an ok feature for you? we've implemented this in our app but I wonder whether this should be a standard feature

Suggested solution

implement it as default feature. I can provide a PR

Alternative

No response

Additional context

No response

Validations

• Follow our Code of Conduct
• Read the Wikis.
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

[ghiscoding]

I've never heard of the F2 shortcut key, is that an Excel shortcut of was is it? Do you have any links that I can read?

[zewa666]

yep, its an Excel feature. also the windows explorer uses it to toggle the editor to rename a selected file.

[ghiscoding]

lol I just replied to an issue on 6pac/slickgrid repo, some people seem to think that key combos are just supposed to magically work in the lib, but the fact is that someone had to implement these combos lol 😅 ...

I don't mind adding more shortcuts as long as it doesn't clash with current implementations and that it's a common Excel shortcut, the last time I've reviewed and revamp key combos was in PR #1244 and I've added many Cypress tests to cover these changes, so if adding F2 shortcut doesn't fail and interfere with these tests then I guess we should be good. I'm guessing that it's going to be a code change in this SlickGrid function.

Side note, as you saw in my release last Friday, I don't think that there would be more releases for v9.x, so this change would have to be introduced in v10 (which is currently the next branch)

slickgrid-universal/packages/common/src/core/slickGrid.ts

Lines 5752 to 5832 in 6d8c738

	protected handleKeyDown(e: KeyboardEvent & { originalEvent: Event }): void {
	const retval = this.triggerEvent(this.onKeyDown, { row: this.activeRow, cell: this.activeCell }, e);
	let handled: boolean | undefined | void = retval.isImmediatePropagationStopped();
	if (!handled) {
	if (this._options.enableCellNavigation && e.ctrlKey && e.key.toLowerCase() === 'c' && !this._options.enableExcelCopyBuffer) {
	// Ctrl+C (copy cell to clipboard, unless Excel Copy Buffer is enabled)
	copyCellToClipboard({
	grid: this as unknown as SlickGrid,
	cell: this.activeCell,
	row: this.activeRow,
	column: this.columns[this.activeCell],
	dataContext: this.getDataItem(this.activeRow),
	});
	} else if (!e.shiftKey && !e.altKey) {
	// editor may specify an array of keys to bubble
	if (this._options.editable && this.currentEditor?.keyCaptureList) {
	if (this.currentEditor.keyCaptureList.indexOf(e.which) > -1) {
	return;
	}
	}
	if (e.ctrlKey && e.key === 'Home') {
	this.navigateTopStart();
	} else if (e.ctrlKey && e.key === 'End') {
	this.navigateBottomEnd();
	} else if (e.ctrlKey && e.key === 'ArrowUp') {
	this.navigateTop();
	} else if (e.ctrlKey && e.key === 'ArrowDown') {
	this.navigateBottom();
	} else if ((e.ctrlKey && e.key === 'ArrowLeft') || (!e.ctrlKey && e.key === 'Home')) {
	this.navigateRowStart();
	} else if ((e.ctrlKey && e.key === 'ArrowRight') || (!e.ctrlKey && e.key === 'End')) {
	this.navigateRowEnd();
	}
	}
	}
	if (!handled) {
	if (!e.shiftKey && !e.altKey && !e.ctrlKey) {
	if (e.key === 'Escape') {
	if (!this.getEditorLock()?.isActive()) {
	return; // no editing mode to cancel, allow bubbling and default processing (exit without cancelling the event)
	}
	this.cancelEditAndSetFocus();
	} else if (e.key === 'PageDown') {
	this.navigatePageDown();
	handled = true;
	} else if (e.key === 'PageUp') {
	this.navigatePageUp();
	handled = true;
	} else if (e.key === 'ArrowLeft') {
	handled = this.navigateLeft();
	} else if (e.key === 'ArrowRight') {
	handled = this.navigateRight();
	} else if (e.key === 'ArrowUp') {
	handled = this.navigateUp();
	} else if (e.key === 'ArrowDown') {
	handled = this.navigateDown();
	} else if (e.key === 'Tab') {
	handled = this.navigateNext();
	} else if (e.key === 'Enter') {
	if (this._options.editable) {
	if (this.currentEditor) {
	// adding new row
	if (this.activeRow === this.getDataLength()) {
	this.navigateDown();
	} else {
	this.commitEditAndSetFocus();
	}
	} else {
	if (this.getEditorLock()?.commitCurrentEdit()) {
	this.makeActiveCellEditable(undefined, undefined, e);
	}
	}
	}
	handled = true;
	}
	} else if (e.key === 'Tab' && e.shiftKey && !e.ctrlKey && !e.altKey) {
	handled = this.navigatePrev();
	}
	}

[zewa666]

hah yeah tell me about those auto-magic features 😅

we're having a few Excel powerusers who naturally compare everything they see to it. so with those little wins its a way to drive more acceptance.

yeah saw that message about last commit for 9x branch and thats totally fine. I'll have it in the backlog for once next becomes the new major and will create a PR than

[ghiscoding]

I'll have it in the backlog for once next becomes the new major and will create a PR than

which should be soon after Angular Standalone and Zoneless are merged. I didn't merge the Zoneless simply because I think there would be many git conflicts between these 2 PRs (but Zoneless should be a straightforward feature, just a benefit to users who are aiming for Zoneless)

[ghiscoding]

@zewa666 ok both Standalone and Zoneless were merged, you can go ahead with your changes. Also a reminder about the other one mentioned in the Standalone PR for

perhaps while at it, we could offer a customClass binding to the slickgridContainer-xyz div, so that its easier to attach custom css classes to the inside of the component

[zewa666]

hmmm so I was trying to figure out the best place to put this change in. In userland I do

this.angularGrid.slickGrid.onKeyDown.subscribe((e, _args) => {
      if (e.which === 113) {
        // 113 is the keyCode for F2
        const activeCell = this.angularGrid.slickGrid.getActiveCell();
        if (activeCell) {
          this.angularGrid.slickGrid.gotoCell(
            activeCell.row,
            activeCell.cell,
            true
          );
          e.preventDefault();
        }
      }
    });

now looking at the slickGrid.ts file I saw the handleKeyDown handler and thought about adding something along the lines of

} else if (e.key === 'F2') {
        if (this._options.editable && !this.currentEditor) {
          this.makeActiveCellEditable(undefined, undefined, e);
          handled = true;
        }
      }

but funny enough this handler is never reached. I also found the same handler in the new hybridSelection mode, but same issue there. So that makes me thinking these are only the handlers that are taking care of cases where the cell already has an active editor. Where would you put that part in to handle keydowns on cells which currently don't have the editor open but can potentially be edited?

[ghiscoding]

there are some conditions that might be hit before it reaches your condition, so I think you simply need to move your condition higher to the top. I would add console logs, or breakpoint, to confirm

[zewa666]

i did so but not entering the handler at all

[ghiscoding]

hmm you might (probably) need the enableCellNavigation for that one to work.

slickgrid-universal/packages/common/src/core/slickGrid.ts

Line 5757 in 6d8c738

if (this._options.enableCellNavigation && e.ctrlKey && e.key.toLowerCase() === 'c' && !this._options.enableExcelCopyBuffer) {

[zewa666]

well I'm stupid ... placed the else if in a wrong slot that was never been able to reach 🤦