Skip to content

fix: change which element keydown is bound to from document to injection div #8188

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.

Sign up for GitHub

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

Merged
cpcallen merged 4 commits into from
May 31, 2024
Merged

fix: change which element keydown is bound to from document to injection div #8188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
935c65a
Change which element keydown is bound to
mark-friedman May 30, 2024
64eb371
Modify keydown tests
mark-friedman May 30, 2024
afa670c
Use browserEvents.conditionalBind()
mark-friedman May 30, 2024
7a3a177
Remove commented out code.
mark-friedman May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions core/inject.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export function inject(
common.setMainWorkspace(workspace);
});

browserEvents.conditionalBind(subContainer, 'keydown', null, onKeyDown);

return workspace;
}

Expand Down Expand Up @@ -320,8 +322,6 @@ let documentEventsBound = false;
* Most of these events should be bound to the SVG's surface.
* However, 'mouseup' has to be on the whole document so that a block dragged
* out of bounds and released will know that it has been released.
* Also, 'keydown' has to be on the whole document since the browser doesn't
* understand a concept of focus on the SVG image.
*/
function bindDocumentEvents() {
if (!documentEventsBound) {
Expand All @@ -333,7 +333,6 @@ function bindDocumentEvents() {
}
}
});
browserEvents.conditionalBind(document, 'keydown', null, onKeyDown);
// longStop needs to run to stop the context menu from showing up. It
// should run regardless of what other touch event handlers have run.
browserEvents.bind(document, 'touchend', null, Touch.longStop);
Expand Down
29 changes: 15 additions & 14 deletions tests/mocha/keydown_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ suite('Key Down', function () {
setup(function () {
sharedTestSetup.call(this);
this.workspace = Blockly.inject('blocklyDiv', {});
this.injectionDiv = this.workspace.getInjectionDiv();
});
teardown(function () {
sharedTestTeardown.call(this);
Expand All @@ -42,7 +43,7 @@ suite('Key Down', function () {
const name = opt_name ? opt_name : 'Not called when readOnly is true';
test(name, function () {
this.workspace.options.readOnly = true;
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.hideChaffSpy);
});
}
Expand All @@ -56,7 +57,7 @@ suite('Key Down', function () {
);
});
test('Simple', function () {
document.dispatchEvent(this.event);
this.injectionDiv.dispatchEvent(this.event);
sinon.assert.calledOnce(this.hideChaffSpy);
});
runReadOnlyTest(createKeyDownEvent(Blockly.utils.KeyCodes.ESC));
Expand All @@ -68,7 +69,7 @@ suite('Key Down', function () {
});
test('Not called on hidden workspaces', function () {
this.workspace.isVisible_ = false;
document.dispatchEvent(this.event);
this.injectionDiv.dispatchEvent(this.event);
sinon.assert.notCalled(this.hideChaffSpy);
});
});
Expand All @@ -92,7 +93,7 @@ suite('Key Down', function () {
const testCaseName = testCase[0];
const keyEvent = testCase[1];
test(testCaseName, function () {
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.hideChaffSpy);
sinon.assert.calledOnce(this.deleteSpy);
});
Expand Down Expand Up @@ -143,7 +144,7 @@ suite('Key Down', function () {
const testCaseName = testCase[0];
const keyEvent = testCase[1];
test(testCaseName, function () {
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.copySpy);
sinon.assert.calledOnce(this.hideChaffSpy);
});
Expand All @@ -164,7 +165,7 @@ suite('Key Down', function () {
const keyEvent = testCase[1];
test(testCaseName, function () {
sinon.stub(Blockly.Gesture, 'inProgress').returns(true);
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.copySpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand All @@ -179,7 +180,7 @@ suite('Key Down', function () {
sinon
.stub(Blockly.common.getSelected(), 'isDeletable')
.returns(false);
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.copySpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand All @@ -192,7 +193,7 @@ suite('Key Down', function () {
const keyEvent = testCase[1];
test(testCaseName, function () {
sinon.stub(Blockly.common.getSelected(), 'isMovable').returns(false);
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.copySpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand Down Expand Up @@ -234,7 +235,7 @@ suite('Key Down', function () {
const testCaseName = testCase[0];
const keyEvent = testCase[1];
test(testCaseName, function () {
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.undoSpy);
sinon.assert.calledWith(this.undoSpy, false);
sinon.assert.calledOnce(this.hideChaffSpy);
Expand All @@ -248,7 +249,7 @@ suite('Key Down', function () {
const keyEvent = testCase[1];
test(testCaseName, function () {
sinon.stub(Blockly.Gesture, 'inProgress').returns(true);
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.undoSpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand Down Expand Up @@ -301,7 +302,7 @@ suite('Key Down', function () {
const testCaseName = testCase[0];
const keyEvent = testCase[1];
test(testCaseName, function () {
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.calledOnce(this.redoSpy);
sinon.assert.calledWith(this.redoSpy, true);
sinon.assert.calledOnce(this.hideChaffSpy);
Expand All @@ -315,7 +316,7 @@ suite('Key Down', function () {
const keyEvent = testCase[1];
test(testCaseName, function () {
sinon.stub(Blockly.Gesture, 'inProgress').returns(true);
document.dispatchEvent(keyEvent);
this.injectionDiv.dispatchEvent(keyEvent);
sinon.assert.notCalled(this.redoSpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand Down Expand Up @@ -343,14 +344,14 @@ suite('Key Down', function () {
);
});
test('Simple', function () {
document.dispatchEvent(this.ctrlYEvent);
this.injectionDiv.dispatchEvent(this.ctrlYEvent);
sinon.assert.calledOnce(this.undoSpy);
sinon.assert.calledWith(this.undoSpy, true);
sinon.assert.calledOnce(this.hideChaffSpy);
});
test('Not called when a gesture is in progress', function () {
sinon.stub(Blockly.Gesture, 'inProgress').returns(true);
document.dispatchEvent(this.ctrlYEvent);
this.injectionDiv.dispatchEvent(this.ctrlYEvent);
sinon.assert.notCalled(this.undoSpy);
sinon.assert.notCalled(this.hideChaffSpy);
});
Expand Down