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

Less scary message with Ctrl-C, remove gcode viewer for android #277

Merged
merged 1 commit into from
Feb 15, 2014

Conversation

jamesgao
Copy link
Contributor

@jamesgao jamesgao commented Oct 9, 2013

Let's try this again, this time on the devel branch!

On all android devices I've tried this on, the gcode viewer crashes the browser very hard because there's not enough RAM on the devices to load up large gcode files. This short snippet simply removes the gcode-related elements if it detects an android browser.

While I realize that this is sub-optimal in the off chance you're running android on a full blown computer, all devices I've tried this on from a brand new Moto X to a Nexus 10 tablet has managed to crash after loading the gcode viewer. Thoughts on this change?

@savorywatt
Copy link
Contributor

I'm not crazy about simply removing it based on what browser. I'd prefer a warning or asking them to confirm / modify a setting rather than a silent fail.

@foosel
Copy link
Member

foosel commented Oct 20, 2013

Yeah, that (among time issues, it's been a bit crazy lately) is one of the reasons I haven't merged it yet. I see the necessity, I'm not sure about the best way to solve that issue though.

One way I thought of would be to somehow tie it to the existing setting for en/disabling the visualizer. That one's server side though, so I'm not that certain yet how exactly to go about this without running into a whole lot of other issues.

@benpye
Copy link

benpye commented Nov 1, 2013

Have you tried it on high end devices with Chrome? I don't see say the Nexus 5 or Nexus 4 having issues, then again I may be wrong.

@jamesgao
Copy link
Contributor Author

jamesgao commented Nov 1, 2013

As I mentioned, I've seen the crash on a Nexus 10 and a Moto X, both very new devices. I've been trying to integrate a checkbox option into the settings that allows you to toggle the feature, but I haven't really had the time.

@foosel foosel merged commit 4581722 into OctoPrint:devel Feb 15, 2014
@foosel
Copy link
Member

foosel commented Feb 15, 2014

Sorry for this staying open so long. I just now merged it onto devel, but removed the Android detection stuff, since with commit ad556e7 the gcode viewer now is a bit more intelligent and refuses to render large gcode files directly (different thresholds for mobile and desktop browsers), instead displaying an "are your really sure you want to do this, this may crash things" message to the user.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2020
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.

4 participants