Shift + Spacebar should move back to the previous slide
- 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