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 touch device support #21

Merged
merged 1 commit into from
Sep 25, 2014
Merged

Conversation

bradenmacdonald
Copy link
Contributor

This adds touch support, because the dragging and dropping was not working on mobile devices. Uses jQuery UI Touch Punch to make this work.

I've tested and confirmed it works in:

  • Mobile Safari (iOS 8) on iPad with xblock in workbench
  • Mobile Safari (iOS 8) on iPad with xblock in LMS
  • Desktop Safari 7.1 with xblock in LMS
  • Desktop Chrome in LMS and workbench

Note: in order to connect to the devstack from an iPad, I had to modify lms/envs/devstack.py on the computer and set SITE_NAME = '192.168.1.xx:8000' rather than SITE_NAME = 'localhost:8000' - otherwise the XBlock wasn't loading correctly.

'public/js/drag_and_drop.js',
)
for css_url in CSS_URLS:
fragment.add_css_url(self.runtime.local_resource_url(self, css_url))
Copy link
Contributor

Choose a reason for hiding this comment

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

self.runtime.local_resource_url appends settings.SITE_NAME from lms configuration to resource url. Devstack have SITE_NAME set to localhost:8000, which obviously does not work when trying to access lms from mobile device. It shouldn't be a problem in production, since SITE_NAME should point to lms domain name anyway, but it could make sense to either:

  1. Use some other way of appending resources: namely, anyone that results in relative links for resources (I'm not yet aware of any, @antoviaque, @dragonfi, @aboudreault or @mtyaka to the rescue :))
  2. Assemble some manual on making it available for external devices (in my case it was just setting SITE_NAME to IP of the computer running lms)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried option 2 - I mentioned this in this PR description ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

@bradenmacdonald yes, I've seen that, I was about to say it could be helpful in future to have this bit of information in documentation somewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

@e-kolpakov +1 - that can be helpful to add to the newcomer documentation, feel free to send a PR there, they are always appreciated and useful : )

@antoviaque
Copy link
Contributor

Nice short fix btw! 👍

antoviaque added a commit that referenced this pull request Sep 25, 2014
@antoviaque antoviaque merged commit 1e99a60 into openedx:master Sep 25, 2014
@bradenmacdonald bradenmacdonald deleted the touch-support branch September 26, 2014 23:55
pomegranited pushed a commit that referenced this pull request Apr 3, 2018
…handling

When dragging, scroll target image as needed
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