Skip to content

Commit

Permalink
fix: prevent scrolling page on element focus
Browse files Browse the repository at this point in the history
  • Loading branch information
straker committed Apr 6, 2024
1 parent 8478eea commit f255333
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 11 deletions.
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@
"vinyl-source-stream": "^2.0.0"
},
"lint-staged": {
"*.js": "eslint --fix"
"*.js": [
"prettier --write",
"eslint --fix"
]
}
}
10 changes: 8 additions & 2 deletions src/button.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { SpriteClass } from './sprite.js';
import Text from './text.js';
import { track } from './pointer.js';
import { srOnlyStyle, noop, addToDom } from './utils.js';
import {
srOnlyStyle,
scrollParams,
noop,
addToDom
} from './utils.js';

/**
* An accessible button. Supports screen readers and keyboard navigation using the <kbd>Tab</kbd> key. The button is automatically [tracked](api/pointer#track) by the pointer and accepts all pointer functions, but you will still need to call [initPointer](api/pointer#initPointer) to have pointer events enabled.
Expand Down Expand Up @@ -189,7 +194,8 @@ class Button extends SpriteClass {
*/
this.focused = true;
// prevent infinite loop
if (document.activeElement != this._dn) this._dn.focus();
if (document.activeElement != this._dn)
this._dn.focus(scrollParams);

this.onFocus();
}
Expand Down
15 changes: 10 additions & 5 deletions src/scene.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { getContext } from './core.js';
import { GameObjectClass } from './gameObject.js';
import { on, off } from './events.js';
import { srOnlyStyle, addToDom, removeFromArray } from './utils.js';
import {
srOnlyStyle,
scrollParams,
addToDom,
removeFromArray
} from './utils.js';
import { collides } from './helpers.js';

/**
Expand Down Expand Up @@ -201,13 +206,13 @@ class Scene {
x,
y,
width,
height,
height
});

if (!this._dn.isConnected) {
addToDom(this._dn, canvas);
}
}
};

if (this.context) {
this._i();
Expand Down Expand Up @@ -280,9 +285,9 @@ class Scene {
// find first focusable object
let focusableObject = this._o.find(object => object.focus);
if (focusableObject) {
focusableObject.focus();
focusableObject.focus(scrollParams);
} else {
this._dn.focus();
this._dn.focus(scrollParams);
}

this.onShow();
Expand Down
2 changes: 2 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ export let noop = () => {};
// style used for DOM nodes needed for screen readers
export let srOnlyStyle =
'position:absolute;width:1px;height:1px;overflow:hidden;clip:rect(0,0,0,0);';
// prevent focus from scrolling the page
export let scrollParams = { preventScroll: true };

/**
* Append a node directly after the canvas and as the last element of other kontra nodes.
Expand Down
6 changes: 5 additions & 1 deletion test/unit/button.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,11 @@ describe('button', () => {
sinon.spy(button._dn, 'focus');
button.focus();

expect(button._dn.focus.called).to.be.true;
expect(
button._dn.focus.calledWith(
sinon.match({ preventScroll: true })
)
).to.be.true;
});

it('should not focus the DOM node if it is already focused', () => {
Expand Down
17 changes: 15 additions & 2 deletions test/unit/scene.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import Scene, { SceneClass } from '../../src/scene.js';
import { _reset, init, getContext, getCanvas } from '../../src/core.js';
import {
_reset,
init,
getContext,
getCanvas
} from '../../src/core.js';
import { emit } from '../../src/events.js';
import { noop, srOnlyStyle } from '../../src/utils.js';
import { collides } from '../../src/helpers.js';
Expand Down Expand Up @@ -219,9 +224,15 @@ describe('scene', () => {
});

it('should focus the DOM node', () => {
sinon.spy(scene._dn, 'focus');
scene.show();

expect(document.activeElement).to.equal(scene._dn);
expect(
scene._dn.focus.calledWith(
sinon.match({ preventScroll: true })
)
).to.be.true;
});

it('should focus the first focusable object', () => {
Expand All @@ -231,7 +242,9 @@ describe('scene', () => {
scene.add(object);
scene.show();

expect(object.focus.called).to.be.true;
expect(
object.focus.calledWith(sinon.match({ preventScroll: true }))
).to.be.true;
});

it('should call onShow', () => {
Expand Down

0 comments on commit f255333

Please sign in to comment.