-
-
Notifications
You must be signed in to change notification settings - Fork 531
[#1481] Improvements to Preset selection menu #1505
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
Conversation
|
Need confirmation of intended functionality. Using the A simple model rocket example, open the Parachute configuration window. Left-click Select preset and chose (highlight) the Estes PK-12 parachute by way of a single left-click on the parachute description (not on the check box), then left-click Close. Previously, this would change the current parachute to the Estes PK-12, but that does not happen now. Instead, you have two choices:
If this is the desired behavior, this PR functions as described with no anomalous behavior (parachutes or other components). |
|
Hmm, this is not so simple. Regardless of whether that's the intended behavior, it's no good. Selecting an item from the database must make that the active component. Basically, this UI has several required functions:
From Craig's description above, function # 1 is not working well here, unless I'm misunderstanding. And function # 3 really has no place to live here: if the preset menu is truly dedicated to selecting from pre-designated components, then it doesn't make sense to put the current selection in the combobox if it is not a preset. Adding "Recent selections" section to the top of the preset menu might clean a lot of this up, but I'm not sure if we should go that way. That's a common approach in things like font menus, where there are a lot of choices. But it's still not perfect. I definitely agree with Sibo's dissatisfaction from the current UI, but I'm not sure this gets us there. I need to think on it more. Let's not rush into this one. BTW the same UI would apply to the texture selector in the Appearance tab. |
|
After reviewing everything, and looking at current functionality and how to expand that, I think I have a proposal that may speak to all of the concerns; it preserves most of the current functionality (eliminating the disavowed Show all compatible). It has three usages:
I think this speaks to everyone's concerns with simple and straight-forward functionality, and functionality which for the most part will be familiar to users. Thoughts @SiboVG and @neilweinstock? |
|
My thoughts:
So where does that leave us? I'm not sure. Sibo and Craig's proposals are not far apart, and my comments above are only slight tweaks. Perhaps we are close. |
|
Is this what you are describing @neilweinstock ? Insofar as the Component name and part number are concerned, I would not take any more space away from the Component name field. The function is that when a component is selected, the component's description becomes the name of the component and the part number would now become the preset name. In my view, part numbers should be kept short, and simply because someone makes a long one should not affect the length of the drop-down preset field. Further thoughts @SiboVG and @neilweinstock, others? |
Yes, although I'm not sure if "apply"/"cancel" is right under all circumstances. For instance, in the config window, you can switch away from it (but selecting another component) and changes are expected to stick even if you never hit "apply". It's kind of weird in that way. Possibly, what we need is simply a button to undo the changes that were made since that window was opened. I'm not sure what that should be called.
Well, we're using a pre-existing database that has some long part numbers in it. To some extent that is out of our control. I haven't tried to see what the longest part number is. Here's a biggie: (In the current implementation, if a long part number is selected, the combobox will expand to show it, which is good: Also, I personally don't use the component name to hold a part description; rather I use it to describe the function of the component (e.g. "Payload section", "motor mount", etc.) This is much more useful to me when looking at the component tree. However, the full description of the part from the database is very useful as well, since part numbers are frequently cryptic (the Apogee 14811 is... what again?) It would be good if it were available in the config dialog (in a read-only field), only when a preset part is currently active. Come to think of it, it would be good to have in the preset pull-down as well, because selecting from a list of cryptic part numbers is not super useful. I had this problem with my selection of Estes BT presets using the legacy database: non-descriptive part numbers meant I basically had to memorize them to use the preset menu, and I struggled with it regularly. I'm not sure the best way to implement this, though... putting full part numbers and descriptions in the preset menu would make it very wide and messy. Maybe put the description in tooltips for the menu? I don't know. I apologize for continuing to add new issues to this but the truth is that this area of the UI currently has all sorts of little problems, not theoretical but actual ones that I encounter all the time. None are fatal, but collectively they make the program slower and harder to use. It is good to consider the whole scope of the problem when trying to fix it. |
I believe that this labeling structure would resolve your concern. If you left-click Undo then the changes revert and the buttons change back to "Apply" (greyed out) and "Close." Left-clicking the "X" in the right-hand corner is treated as. . . what. . . "Accept", or "Undo" and close (logic says the latter)? Does this work for you @neilweinstock, others? |
|
It is generally not a good idea for button labels to change like that (there are occasions when it is appropriate, but I don't think this is one of them.) If we want a separate Undo and Close functions, we should just have an additional button which is only enabled at the appropriate time. I confess I'm not really sure what the function of the "Apply" button here? It would be a major change to the program (and not a desirable one) to require the user to hit "apply" every time they wanted to commit changes. |
|
Apply implements the changes AND closes the window. The reason undo and close have the same button is because it is essentially the same function as the “X”, to undo changes then close the window without having to relocate the mouse, just two slow left clicks. We could use “cancel” and “update” like GitHub… |
|
Have one last idea… I’ll mock it up tonight or tomorrow. Greater simplicity. |
|
The mock-up will have some of the same functionality as GitHub… |
Before you do... please explain why we need to do anything more than just add a "revert" button? |
|
Huh, your screen grab says “apply”… just teasing. |
|
Let's start with current functionality, with the added "preset" drop-down created by @SiboVG, ignoring labels for the moment. In analyzing the functionality, a characteristic change on the component configuration pane is given effect immediately (closing the pane is not required). And, for comparison, Undo (Ctrl+Z), to use @neilweinstock's nomenclature, "reverts" all of the user's changes made to the data that existed before the first of those changes were made. Functionality at issue:
Currently, once changed, the past data may only be restored by using the Undo (Ctrl+Z) function. The last question posed by @neilweinstock was:
For the sake of consistency, the following should be the beginning point for a functionality discussion:
Now, let's talk about labels.
And, I agree with that nomenclature as well. So, I suggest that Label 1 be "Custom".
Taken together, this is my suggestion for changes to the component configuration pane:
As to the parts library/preset pane, I still believe that the functionality described above is best, see:
|
I agree with changes 1-2. However, your propositions for 3, 4 and 5 is a duplicate of #960. I also prefer an 'Ok' / 'Cancel' button over the 'Update/Close' / 'Cancel' button. So since numbers 3-5 are already documented in #960 and since that functionality will require a bit of extra programming, I will not implement them in this PR. |
|
Last commit updates the labels. |
|
No functional anomalies found. The following configuration panes truncate the "Custom" label:
However, not all truncations are consistent, some, like the body tube, come and go. Build 784 |
|
I would also change the label "Close" to "OK" now, rather than waiting for Issue #960 to be completed. |
|
I vote to keep it at "Close", which precisely describes what the button does. "OK" isn't bad necessarily, but it's not clearly better and therefore there is no need to change it. It is important to remember that, when comparing this dialog to things like Github or whatever, that ours if fundamentally different because changes take place and "stick" immediately. So labels that are different from what you would find elsewhere are often appropriate because they function differently. Our close button does not do the same thing as the typical "OK" button. It just closes the dialog. As for the other proposed changes... will wait to argue that with issue #960, post-release. |
|
Last commit fixes the dropdown menu text being cut off (already tested by @hcraigmiller). |
|
Functions as described, no anomalies found. Build 792 |
|
Aha, now I know why I filed #1518, because I did my testing for that issue on an OR instance from this PR. So currently, this PR breaks the functionality of 'selecting a preset without checking it applies that preset either way'. I'll look into it... |
|
I had a suspicion that might be the case.... :) |
|
Should be fixed now. |












This is my proposal to fix #1481. Two things I hated about the previous selection menu:
So my proposal is:
Additionally, this PR also increases the maximum number of items in the selection menu from 7 to 25.
I also changed the text underneath the preset dialog from 'Select to add preset to drop-down menu' to 'Check to add preset to the preset drop-down menu in the component edit dialog', as for new users, the first text could still be very confusing.
Demo:
Screen.Recording.2022-06-30.at.17.01.14.mp4