Skip to content

Commit 80e3c45

Browse files
committed
Remove up/down to flip keyboard shortcut
This gets confusing when combined with slider focus, where the shortcut does potentially the opposite of what it appears to do.
1 parent 67c508f commit 80e3c45

File tree

3 files changed

+17
-24
lines changed

3 files changed

+17
-24
lines changed

src/BookReader.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -876,18 +876,14 @@ BookReader.prototype.setupKeyListeners = function () {
876876
e.preventDefault();
877877
this.last();
878878
break;
879-
case "ArrowDown":
880879
case "PageDown":
881-
case "Down": // hack for IE and old Gecko
882880
// In 1up and thumb mode page scrolling handled by browser
883881
if (this.constMode2up === this.mode) {
884882
e.preventDefault();
885883
this.next();
886884
}
887885
break;
888-
case "ArrowUp":
889886
case "PageUp":
890-
case "Up": // hack for IE and old Gecko
891887
// In 1up and thumb mode page scrolling handled by browser
892888
if (this.constMode2up === this.mode) {
893889
e.preventDefault();

src/BookReader/Navbar/Navbar.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,23 @@ export class Navbar {
328328
'aria-valuemax': br.book.getNumLeafs(),
329329
});
330330

331+
// Ignore up/down arrow keys and page up/down keys, since they're confusingly different
332+
// between slider movement and page movement. Down decreases slider value, which would move
333+
// the scroll _up_ in 1up.
334+
$sliders.find('.ui-slider-handle').off('keydown').on('keydown', function (event) {
335+
switch (event.keyCode || event.which) {
336+
case $.ui.keyCode.UP:
337+
case $.ui.keyCode.DOWN:
338+
case $.ui.keyCode.PAGE_UP:
339+
case $.ui.keyCode.PAGE_DOWN:
340+
return;
341+
default:
342+
// Forward along to the default handler for other keys
343+
$.ui.slider.prototype._handleEvents.keydown.call($(this).parent().data('ui-slider'), event);
344+
return;
345+
}
346+
});
347+
331348
$sliders.on('slide', (event, ui) => this.updateNavPageNum(ui.value));
332349

333350
$sliders.on('slidechange', (event, ui) => {

tests/jest/BookReader.keyboard.test.js

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,6 @@ describe('Keyboard shortcuts', () => {
7373
expect(keyEvent.preventDefault).toHaveBeenCalledTimes(1);
7474
});
7575

76-
test('ArrowDown key', () => {
77-
br.mode = br.constMode2up;
78-
br.next = jest.fn();
79-
const keyEvent = new KeyboardEvent('keydown', {'key': 'ArrowDown'});
80-
keyEvent.preventDefault = jest.fn();
81-
document.dispatchEvent(keyEvent);
82-
expect(br.next).toHaveBeenCalledTimes(1);
83-
expect(keyEvent.preventDefault).toHaveBeenCalledTimes(1);
84-
});
85-
8676
test('PageDown key', () => {
8777
br.mode = br.constMode2up;
8878
br.next = jest.fn();
@@ -93,16 +83,6 @@ describe('Keyboard shortcuts', () => {
9383
expect(keyEvent.preventDefault).toHaveBeenCalledTimes(1);
9484
});
9585

96-
test('ArrowUp key', () => {
97-
br.mode = br.constMode2up;
98-
br.prev = jest.fn();
99-
const keyEvent = new KeyboardEvent('keydown', {'key': 'ArrowUp'});
100-
keyEvent.preventDefault = jest.fn();
101-
document.dispatchEvent(keyEvent);
102-
expect(br.prev).toHaveBeenCalledTimes(1);
103-
expect(keyEvent.preventDefault).toHaveBeenCalledTimes(1);
104-
});
105-
10686
test('PageUp key', () => {
10787
br.mode = br.constMode2up;
10888
br.prev = jest.fn();

0 commit comments

Comments
 (0)