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

Add classy hammer time a version of hammer time which works of class … #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arschmitz
Copy link
Contributor

@arschmitz arschmitz commented Sep 29, 2016

…names instead of the style prop

This allows a much smaller payload in SPA's and also reduces the size of hammer-time,
but it also moves away from being a polyfill a little

Questions

  • Should we install a 2 line style sheet with native touch action rules when there is native support?
  • Should the class options be arrays instead of strings allowing multiple classes per touch-action type?
  • Should we abstract the core methods and make MO / Class an extension to core Already decided to do this

…names instead of the style prop

This allows a much smaller payload in SPA's and also reduces the size of hammer-time,
but it also moves away from being a polyfill a little
document.documentElement.style[ "-ms-touch-action" ];

// If there is native touch action bail the hammer has already dropped
if ( nativeTouchAction || !touchevents || !MO ) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should remove the !MO from here

@pzuraq
Copy link

pzuraq commented Oct 4, 2016

Looks good other than the one bug, think this will work for class-based hammertime 😄

@pzuraq
Copy link

pzuraq commented Oct 4, 2016

Regarding the questions:

  1. I'd say yes, the expected behavior using hammertime is that these classes will provide that functionality. It should probably be a separate file though, not part of the install method, or overridable via a boolean at least so that users can turn it off if they decide to (which would be weird, but still).
  2. I don't think so, convention > configuration. Give users one way to specify the class, it's more performant that way too (though we could convert the array to a map in the install step, meaning it would be about as performant). We can always add that functionality in a minor version bump later on.
  3. Yes, the logic is very similar so it makes sense to abstract them out. Ideally we would still be able to only include the one we want to actually use in final builds though, may require an additional build step.

@runspired
Copy link
Collaborator

Should we install a 2 line style sheet with native touch action rules when there is native support?

I was going to auto-add something like this for ember-hammertime.

.hammer-time {
  touch-action: manipulation;
  -ms-touch-action: manipulation;
  cursor: pointer;
}

Should the class options be arrays instead of strings allowing multiple classes per touch-action type?

maybe just some presets for the 3 we support?

.ht-manipulation
.ht-none
.ht-auto

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.

3 participants