Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Fixes issue #292 and the uniqueID method #1001

Merged
merged 1 commit into from
Jun 30, 2014

Conversation

wicol
Copy link
Contributor

@wicol wicol commented Jan 27, 2014

Multiple clones were added for no apparent reason. Also uniqueID only
fixed the id's of sub elements due to the use of find().

@ghost
Copy link

ghost commented Jan 31, 2014

This fix breaks touch events for the slider.

@wicol
Copy link
Contributor Author

wicol commented Jan 31, 2014

In what way? Touching and swiping seems to work fine here on Android/Chrome and iOS/Safari.

@ghost
Copy link

ghost commented Jan 31, 2014

Tested on Android 4.3.1 native browser and OSX/Chrome

@wicol
Copy link
Contributor Author

wicol commented Jan 31, 2014

Ok I just tested on a Nexus 4 with CM10.2 (Android 4.3.1) native browser and that works for me. Not sure how touch events would work on OSX/Chrome..? I tried dragging the images around on the examples here to no avail: http://flexslider.woothemes.com (on OSX/Chrome 32.0.1700.102).

@ghost
Copy link

ghost commented Jan 31, 2014

thanks for the input, i'll throw together a test over the weekend to show the issue.
ps. you can emulate touch events in chrome dev tools.

@wicol
Copy link
Contributor Author

wicol commented Jan 31, 2014

Oh, thanks for the tip, just tried the emulation features in Chrome Canary, very cool :) But when emulating a Nexus 4 the touch still works for me (albeit a bit quirky compared to a real device - it doesn't snap to the correct image (in the official version or with my fix)). If you can create a test to show the issue that would be great!

@ghost
Copy link

ghost commented Feb 3, 2014

@Reec sorry for not testing this earlier, your solution works by default, it was my setup that was causing issues.

@wicol
Copy link
Contributor Author

wicol commented Feb 3, 2014

Glad to hear it! Now let's hope someone gets around to merging this at some point.

@tylernotfound
Copy link
Contributor

This looks good. Can you update this pull request to suit the current code? That way I can give you credit for this one.

Multiple clones were added for no apparent reason. Also uniqueID only
fixed the id's of sub elements due to the use of find().
@wicol
Copy link
Contributor Author

wicol commented Jun 30, 2014

Ok, I reset everything and applied the fix manually again. Should be good to go. Nice to see some work being done on this again!

tylernotfound pushed a commit that referenced this pull request Jun 30, 2014
Fixes issue #292 and the uniqueID method
@tylernotfound tylernotfound merged commit 8b3766e into woocommerce:master Jun 30, 2014
@tylernotfound
Copy link
Contributor

Thanks for this, @Reec!

@wicol
Copy link
Contributor Author

wicol commented Jul 14, 2014

Oh right, if you didn't notice, I didn't update the minified version so you might want to do that.

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

Successfully merging this pull request may close these issues.

2 participants