t-oster / VisiCut

A userfriendly tool to prepare, save and send Jobs to Lasercutters
https://visicut.org
Other
233 stars 114 forks source link

Cannot select laser cutter if starting with none and adding just one. #360

Closed tealvince closed 4 years ago

tealvince commented 8 years ago

If the app is launched without the suggested settings (so no laser cutters are yet defined), then if one creates a single laser cutter afterwards, the laserDeviceComboBox is hidden while the selected laser device is still null. This leaves no way to select a laser cutter or enable the "execute" button without creating a second laser cutter or restarting the program.

I'm not familiar with the code enough to know if there is a better solution, but this is what I did:

diff --git a/src/com/t_oster/visicut/gui/MainView.java b/src/com/t_oster/visicut/gui/MainView.java index a642752..0dcbfc0 100644 --- a/src/com/t_oster/visicut/gui/MainView.java +++ b/src/com/t_oster/visicut/gui/MainView.java @@ -591,6 +591,7 @@ public class MainView extends javax.swing.JFrame this.laserCutterComboBox.setSelectedIndex(0); this.laserCutterComboBox.setVisible(false); this.jLabel9.setVisible(false);

pelrun commented 8 years ago

I think it's better to move the ignoreLaserCutterComboBoxUpdates=false line above that if statement; the this.laserCutterComboBox.setSelectedIndex(0) line should kick off the appropriate update except the flag is suppressing it.

renebohne commented 8 years ago

@tealvince did you test what @pelrun suggested? Did it work and can you provide a pull request for this issue?

jnweiger commented 4 years ago

The behaviour improved with newer versions of visicut:

The behaviour with only one laser now is:

Workaround: add a second dummy laser device, so that users see the exepcted image and name of their laser onscreen.

jnweiger commented 4 years ago

btw. it is hard to see what the suggested solution is, from the provided snippet. All I can decipher is:

+++ b/src/com/t_oster/visicut/gui/MainView.java @@ -591,6 +591,7 @@ public class MainView extends javax.swing.JFrame this.laserCutterComboBox.setSelectedIndex(0); this.laserCutterComboBox.setVisible(false); this.jLabel9.setVisible(false);

* ```
   this.visicutModel1.setSelectedLaserDevice(LaserDeviceManager.getInstance().getAll().get(0));
  ```

  }
  else
  {
mgmax commented 4 years ago

If there is only one laser, then there is nothing to choose, so I think it's correct to not show the dropdown box in that case.

t-oster commented 4 years ago

Yes this is the intended behavior.why display something that cannot be changed?

jnweiger commented 4 years ago

Assume a lab has two lasers. On the laptops used during openlab hous, VisiCut has only one of them configured. The user has no indicator which one.

Other laptops have both configured. By comparing how it looks on the screen, users would assume that no laser is configured, although one is configured.

Presenting it as a drop down, indicates choice. Well, with one element there is no choice. But presenting it ins some (static form) is still helpful to achieve consistency.

Note that starting VisiCut with an empty list of machines displays 'Please select' but nothing can be selected. image

jnweiger commented 4 years ago

Suggestion: https://github.com/t-oster/VisiCut/pull/559 (not fixing the 'Please select' part. that is a rare beast.)

t-oster commented 4 years ago

Hmm... not an easy one. First: Why should in a lab with two lasercutters one laptop have only one lasercutter configured anyway? Imagine a lab with only one lasercutter. There we are showing a UI item which suggests 'Please make a choice' where there is none. The case that the user starts with an empty configuration and has to add a lasercutter himself should be rare.

tealvince commented 4 years ago

A simple workaround I've seen used elsewhere is to always add an "Add lasercutter..." item to the bottom of the menu.

t-oster commented 4 years ago

I don't think we need a fix or work around. The idea is to hide complexity when not necessary. If only one lasercutter is configured I do not see any reason to show it to the user because I assume there is only one in the room. If no lasercutter is configured at all, that may be a special case, but I am not sure if this should happen anyway because either you start with downloaded settings or with the example settings.

t-oster commented 4 years ago

So I checked: If you start with the empty settings and add a lasercutter it is correctly selected (and the combobox is hidden on purpose). So the initial issue is fixed. @jnweiger no offence and thank you for your contributions, but in this case I just disagree and would like to keep it that way.

jnweiger commented 4 years ago

@t-oster no offence taken. We basically discuss UX here. It is a "least surprise" versus "reduce clutter" conflict. Both principles are valid to me. You have to choose. That is perfectly fine here.

I'll bring up more crazy suggestions, no worries :-)

(To answer your question about not configuring all lasers in a lab: Three of the labs I know have Mr.Beam, glowforge and Trotecs; but use VisiCut with their other laser.)