Skip to content

fix gl3d hover on mobile and tablet devices #31

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

Merged
merged 1 commit into from
Oct 30, 2020

Conversation

jdpaterson
Copy link
Contributor

Related to: plotly/plotly.js#4550
Looks to fix interactivity in 3d plots on mobile and tablet devices.
I'm not sure what the best way is to test this and that it doesn't break anything else. But I did test by making these changes on the plotly.js repo at a commit that was before camera.js was moved to this repo.

@archmoj
Copy link
Contributor

archmoj commented Oct 28, 2020

Thanks for the PR.
Are you interested in submitting a PR to plotly.js repo as well?
In any case I could help you with that.

@archmoj
Copy link
Contributor

archmoj commented Oct 28, 2020

@jdpaterson Now to submit a PR to plotly.js all you need is to replace

"gl-plot3d": "^2.4.6",

this line in the package.json file of plotly.jswith

"gl-plot3d": "git://github.com/jdpaterson/gl-?.git#9724b397857dfece7400005dea0e1c51f3ceeb14",

save it; then run npm install
and commit package-lock.json as well as package.json files.

@jdpaterson
Copy link
Contributor Author

@archmoj there are no changes required plotly.js, the updates here should cover it. I can create a PR in plotly.js just for testing purposes if it's easier for you.

@archmoj
Copy link
Contributor

archmoj commented Oct 29, 2020

@archmoj there are no changes required plotly.js, the updates here should cover it. I can create a PR in plotly.js just for testing purposes if it's easier for you.

@jdpaterson
In plotly.js we need to upgrade to a patch including this commit. And yes it would help with testing as well.
So please submit a PR there and mention this PR in the description or alternatively in a comment on the package.json changes.
Thanks.

@jdpaterson
Copy link
Contributor Author

@archmoj understood. Pull request created on Plotly.js: plotly/plotly.js#5239

@archmoj
Copy link
Contributor

archmoj commented Oct 30, 2020

Good catch!
💃

@archmoj archmoj merged commit 80378a4 into gl-vis:master Oct 30, 2020
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