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

fix(reorder-group): should be able to reorder items from within a sha… #17747

Merged

Conversation

Projects
None yet
3 participants
@adamlacombe
Copy link
Contributor

commented Mar 10, 2019

Short description of what this resolves:

Changes proposed in this pull request:

  • reorder-group: createGesture on this.el instead of this.doc.body

Ionic Version: 4.1.1

Fixes: #17746

adamlacombe and others added some commits Mar 11, 2019

@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Mar 25, 2019

Hi there,

Thanks for the PR! Would you be able to add some tests that verify this functionality? Thanks!

liamdebeasi and others added some commits Mar 25, 2019

@adamlacombe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

Sure, I'll give it a shot. I just added a standard interactive test. I'll add onto that to verify this functionality shortly.

adamlacombe added some commits Mar 26, 2019

@adamlacombe

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

@liamdebeasi All set - let me know if you have any questions or concerns. Thanks!

@brandyscarney

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Thanks for the PR! The fix looks good to me. 🙂 @liamdebeasi Could you review the tests?

brandyscarney and others added some commits Mar 26, 2019

@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Hey there,

I pushed a few updates to your branch regarding the tests:

  • simplified the tests a little bit and reorganized some functions
  • sped up the tests (originally they were running at ~2600ms on my machine, got them down to ~700ms)

Going to do another quick look over, but everything else looks really great! Nice work! 🎉

@liamdebeasi
Copy link
Member

left a comment

Thank you!

@brandyscarney brandyscarney merged commit 352797e into ionic-team:master Mar 26, 2019

1 check passed

build Workflow: build
Details
@brandyscarney

This comment has been minimized.

Copy link
Member

commented Mar 26, 2019

Merged 🎉

Kiku-git added a commit to Kiku-git/ionic that referenced this pull request May 16, 2019

fix(reorder-group): add ability to reorder items inside shadow (ionic…
…-team#17747)

- adds ability to reorder items from within a shadow dom component
- adds interactive test

fixes ionic-team#17746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.