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 property for ignoring touch dragging on the slider bar #209

Merged
merged 9 commits into from
Mar 9, 2018

Conversation

NovapaX
Copy link
Contributor

@NovapaX NovapaX commented Feb 22, 2018

Only a tap and a mousedown on the bar should alter the knob-position.
Fixes #178

Only a tap and a mousedown on the bar should alter the knob-position.
Fixes PolymerElements#178
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@NovapaX
Copy link
Contributor Author

NovapaX commented Feb 22, 2018

CLA signed

@googlebot
Copy link

CLAs look good, thanks!

@NovapaX
Copy link
Contributor Author

NovapaX commented Feb 22, 2018

This is a bit more complicated than initially thought.
To accomplish this in a clean way there should be a mousetrack event. So we could just reuse the onTrack function and set on-mousetrack="_onTrack" on the slider bar.
Or someone must have another idea how to accomplish the following behaviour:

  • mouse click on the bar sets position and starts dragging
  • touchmove on the bar does nothing
  • touch tap on the bar sets position

Only thing missing here is the ability to mousedown on the bar anywhere
and start dragging.
@NovapaX
Copy link
Contributor Author

NovapaX commented Feb 22, 2018

oh wow... looks like I can't test the click event.
created a seperate PR for that: PolymerElements/iron-test-helpers#93

@e111077
Copy link
Contributor

e111077 commented Feb 22, 2018

This is a breaking change. Can you please hide this feature under a flag? I'm guessing you can do this in connected callback:

properties = {
  __boundListenFn:  {
    type: Function
  },
  __eventType:  {
    type: string,
    value: 'down'
  },
  /**
   * insert closure doc
   */
  ignoreBarClicks: {
    type: Boolean,
    value: false
  }
}

connectedCallback() {
  ...
  if (this.ignoreBarClicks) {
    // or however you want to implement this
    this.__eventType = 'click';
    this.__boundListenFn = this._barclick.bind(this);
  } else {
    this.__eventType = 'down';
    this.__boundListenFn = this._bardown.bind(this);
  }

  this.$.slderBar.addEventListener(this.__eventType, this.__boundListenFn);
}

disconnectedCallback() {
  ...
  this.$.slderBar.removeEventListener(this.__eventType, this.__boundListenFn);
}

I'll see if i can figure out what's going on with mockInteractions

@NovapaX NovapaX changed the title Stop catching touchdowns on the bar. Add property for ignoring touch dragging on the slider bar Feb 23, 2018
@NovapaX
Copy link
Contributor Author

NovapaX commented Feb 23, 2018

I came up with a working solution that ignores tracking of touches on the slider bar, but allows tapping on the slider bar to set the knob position.
Also was able to enable it all throught the on-[event] attributes so no need to manually set and tear down eventListeners.

@e111077
Copy link
Contributor

e111077 commented Feb 23, 2018

Hmm, I think this will work. I have to run it through an internal suite of tests to see if it breaks anything; we may be able to push this through as a bug fix

@e111077
Copy link
Contributor

e111077 commented Mar 7, 2018

This PR slipped through the cracks. Will prioritize internal testing

@e111077
Copy link
Contributor

e111077 commented Mar 9, 2018

All tests pass. You're golden!

@e111077 e111077 merged commit 20e5b3a into PolymerElements:master Mar 9, 2018
@NovapaX
Copy link
Contributor Author

NovapaX commented Mar 9, 2018

Great! Thanks for your help!

@NovapaX NovapaX deleted the patch-1 branch March 9, 2018 08:18
@NovapaX
Copy link
Contributor Author

NovapaX commented Mar 15, 2018

any idea on whether (and when) this can get pushed into a release?

@e111077
Copy link
Contributor

e111077 commented Mar 15, 2018

We've been moving to a weekly release committee / schedule. Expect to see an update on Friday

@e111077
Copy link
Contributor

e111077 commented Mar 16, 2018

Released a v2.0.5

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

Successfully merging this pull request may close these issues.

3 participants