impress / impress/impress.js

Shift + Spacebar should move back to the previous slide

Open
#710 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
38.2k
Forks
6.6k
PR merge metrics
No merged PRs in 30d

Description

Shift-Space doesn't move back to the previous step on:

```
Google Chrome 70.0.3538.110 (Official Build) (64-bit)
```

`Shift-Tab` works fine and I think that the problem is with the heavily commented logic [inside](https://github.com/impress/impress.js/blob/master/js/impress.js#L3180) the `isNavigationEvent` function.

The second if statement returns true for tab only and the third returns false if shift is pressed. `event.shiftKey` and `event.keyCode === 32 // Shift+space` is then never handled by the code added in https://github.com/impress/impress.js/pull/706/commits/b1e5186c3b995d7e3bcdf1e7428b3f22db55d5ea . PR https://github.com/impress/impress.js/pull/706 was to make `Shift-Space` move back a step and close issue https://github.com/impress/impress.js/issues/248.

I think this could be a one line change:

```
- if ( event.keyCode === 9 ) {
+ if ( event.keyCode === 9 || event.keyCode === 32 ) {
```

But that would leave difficult to follow comments, for example "sole exception" on the line above or:

```
// as another way to moving to next step... And yes, I know that for the sake of
// consistency I should add [shift+tab] as opposite action...
```
When I think `Shift-Tab` was added in 0dc8b436502cde657dda3137c17271e8fb3e87a0.

It might be better to refactor the logic in this function or rewrite the comments. I'd be happy to have a go at a PR for this but I didn't get the test suite running on my first attempt. I use puppeteer for other projects on this setup and merging https://github.com/impress/impress.js/pull/637 as it is now should make writing and running tests easier for me.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in js/impress.js at the isNavigationEvent function and review the keyboard logic and its comments, especially the handling of Shift-Tab and Shift-Space. Get the test suite running with the mentioned Puppeteer setup, then verify that Shift-Space moves to the previous step without regressing existing keyboard navigation.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.