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

[Request] Layer up/down buttons in the G-Code Viewer #1149

Closed
julianrinaldi opened this issue Dec 17, 2015 · 14 comments

Comments

@julianrinaldi
Copy link

commented Dec 17, 2015

It can occasionally be hard to select the exact layer you want in the G-Code viewer. The scroll bars generally work well, but having the ability to click a button to move up and down a layer would be useful and more convenient in certain situations. Having an up single layer button at top of the scroll bar, and a down single layer at the bottom of the scroll bar would be perfect.

@woferry

This comment has been minimized.

Copy link

commented Feb 12, 2016

If possible it would be great to support the up and down keys as well, I'd rather navigate the layers with my keyboard than clicking up/down buttons in the browser window.

@agarwali

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2016

@foosel my team and I are working on OctoPrint and would like to tackle this issue. The bootstrap slider at https://github.com/foosel/OctoPrint/blob/536bb31965db17b969e7c1c53e241ddac4ae1814/src/octoprint/static/js/lib/bootstrap/bootstrap-slider.js does not support this arrow functionality. We could either try to edit the bootstrap-slider.js or we could try to add an extra button functionality on top of the Gcode viewer jinja template. What do you think would be a better approach?

@BillyBlaze

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2016

@agarwali, editing library files is a really bad practice, libraries tend to get upgraded and your work will then get dissolved. Also the bootstrap slider exposes itself on $element.data("slider") and on $element.slider("property")

The best and easiest way is to edit the Gcode viewer jinja template and to add a click binding function in the corresponding viewmodel with something like:

self.nextLayer = function() {
    self.layerSlider.slider("setValue", self.layerSlider.slider("getValue")+1);
}

Salandora pushed a commit that referenced this issue Mar 26, 2016

Salandora Salandora
Added the ability to control the layer slider of the gcodeViewer with…
… the keyboard (Up Arrow, Down Arrow, Page Up, Page Down) as requested in Issue #1149
@Salandora

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2016

This one was bothering me quite a while now.
I tackled this one for getting back into Javascript, it's inside the devel branch.
Move your Mouse over the GcodeViewer and use up arrow, down arrow, page up and page down buttons for a layer change.

Up/Down arrow: +1/-1 Layer
Page up/down: +10/-10 Layers

@agarwali I'm sorry I somehow totaly overread that you wanted to tackle this one -.- shame above me I'm really really sorry for that.

@agarwali

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2016

@Salandora it's ok. So you tackled the key binding issue, but the issue also mentions the implementation of up/down buttons, which I don't see in the devel branch. Have you implemented the buttons or is that not required?

@Salandora

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2016

I haven't added buttons for it, just the keyboard control, I personally like a keyboard solution more than extra buttons.

@agarwali

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2016

@Salandora my group and I tested your changes and it is working. If the button is not required than would this issue be called as resolved or marked as a milestone for a later version?

@Salandora

This comment has been minimized.

Copy link
Collaborator

commented Mar 28, 2016

Well I don't know maybe a button would be interesting for mobile devices.
So I guess for people who dont want to use the touchui plugin it is still an interesting addition.

@foosel

This comment has been minimized.

Copy link
Owner

commented Mar 29, 2016

@agarwali I'd say, ticket asks for a button, and a button actually makes sense for mobile where you don't have a keyboard (basically what @Salandora said), the only question is how it would fit into the UI - and @BillyBlaze should also be involved since he'll need to accommodate any added buttons there in the TouchUI plugin too (I figure there would be some work involved there since the GCODE viewer layout is a bit "meh" and not as straightforward to port and shuffle around as e.g. your regular settings dialog).

@agarwali

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2016

@foosel fitting the button in the UI was indeed a challenge. Below is the screenshot of the best possible thing we could come up with.
screenshot from 2016-04-04 00 18 38
@BillyBlaze the changes were also made in the TouchUI here is what it looks like

We would appreciate your feedback.
@Salandora you javascript onkey function triggers console error when there is no G-Code uploaded because it calls the function trying to increment the disabled slider value. We were thinking of fixing this issue by disabling the event if G-Code is not uploaded.

@Salandora

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2016

@agarwali nice screens.
And good spot if you guys like you can fix it.
As a side note maybe don't disable the whole event just check inside the event if there is a file loaded or not.

@BillyBlaze

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2016

Great job! Does it need a PR in TouchUI or did it work 'out-of-the-box'?

@agarwali

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2016

@Salandora yeah we just checked inside the event. @BillyBlaze it worked out of the box.

@agarwali agarwali referenced this issue Apr 8, 2016

Merged

Dev/gcode layer button #1306

10 of 10 tasks complete
@foosel

This comment has been minimized.

Copy link
Owner

commented Sep 26, 2016

Implemented in current devel by PR #1306

@foosel foosel closed this Sep 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.