Skip to content
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.

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

Spacebar should scroll in list mode #29

Merged
merged 1 commit into from
Mar 15, 2016

Conversation

RubenVerborgh
Copy link
Contributor

I received a report (RubenVerborgh/WebFundamentals#18) that the current spacebar behavior in Shower is non-intuitive. As you know, the space bar on webpages typically has the function of scrolling.

In slide mode, it makes sense to change this behavior, since scrolling does not apply there. This is what Shower currently does.

In list mode, however, scrolling seems to the preferred behavior. Yet Shower alters this behavior to skipping to the next slide (for which there are many other keybindings anyway). This commit fixes that behavior to scrolling in list mode.

Should you have any concerns please let me know. In particular:

  • I used a case fallthrough, which the style file allows me but you might not like.
  • I was not sure if I needed to update the keys.js test (is it full screen or not?), as the tests on master currently don't run.

@pepelsbey
Copy link
Contributor

This is definitely not intuitive, so I’ll merge it with pleasure. I’d stil add a test for it to keys.js even though it’s failing for now so we’ll have chance not to forget about it while fixing tests.

@RubenVerborgh
Copy link
Contributor Author

Fair enough; I just wasn't sure on how to run the keys tests by themselves. If you could explain how I can do that, I'll gladly write a test.

@pepelsbey
Copy link
Contributor

@RubenVerborgh, it’s simple: grunt test and you’ll see:

FAIL 40 tests executed in 2.406s, 30 passed, 10 failed

@pepelsbey
Copy link
Contributor

To run just keys.js change *.js to keys.js here:

    casperjs: {
        files: ['tests/functional/*.js']
    }

The space bar on webpages typically has the function of scrolling.
In slide mode, it makes sense to change this behavior,
since scrolling does not apply there.
In list mode, however, scrolling is the preferred behavior.
@RubenVerborgh
Copy link
Contributor Author

Pushed tests. (2 tests of keys.js still fail, they are unrelated and failed before.)

pepelsbey pushed a commit that referenced this pull request Mar 15, 2016
@pepelsbey pepelsbey merged commit 955ba2e into shower:master Mar 15, 2016
@pepelsbey
Copy link
Contributor

Released in shower-core and shower packages, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants