Skip to content

Conversation

@SiboVG
Copy link
Member

@SiboVG SiboVG commented May 23, 2023

This PR fixes #2079 and adds the stability in percentage length in addition to the stability value in the currently selected stability unit. If the selected unit is already the percentage, or the stability value is N/A, the stability percentage is not included.

Screen.Recording.2023-05-24.at.01.43.07.mp4

@neilweinstock
Copy link
Contributor

Two things:

  1. I don't think there should be a space between the number and the percent sign. You don't say 100 %, you say 100%.
  2. It seems weird to me that if you select Calibers, it also shows percentage. But if you select Percentage, it does not show caliber. Why does it work this way?

@SiboVG
Copy link
Member Author

SiboVG commented May 24, 2023

Two things:

  1. I don't think there should be a space between the number and the percent sign. You don't say 100 %, you say 100%.
  2. It seems weird to me that if you select Calibers, it also shows percentage. But if you select Percentage, it does not show caliber. Why does it work this way?
  1. Well, in theory, there should always be a space between the value and the unit. People are used to writing the percentage sign without a space, but according to scientific notation, the space is correct. Also, how it's currently displayed is just standard unit formatting of OR, it would require a bit more (ugly) coding to change it to exclude the space.
  2. Idk, but this made me think that we should maybe add a "Stability secondary" unit in the preferences so users can choose which extra stability value to show, instead of having it be fixed to the percentage unit?

@neilweinstock
Copy link
Contributor

  1. OK, but a space before the percent sign will not stop looking weird/wrong to me. If it takes some new weird code to remove it, though, then forget it for now.
  2. Simplicity is key. One option would be to always show both, but let the user select which one comes first via the preferences. E.g., "2.0 cal (15 %)" vs. "15 % (2.0 cal)", so they can choose which one is "primary", but we always show both. Is there any significant downside to always showing both? It certainly wouldn't bother me, and given how difficult it is to switch back and forth by going into preferences, leaving both visible all the time seems like a good idea to me, even with the extra space. :)

@neilweinstock
Copy link
Contributor

Another alternative: instead of the parentheses, display like this: "2.0 cal / 15 %" This way, neither is really primary or secondary. You always show both, and remove the preference entirely.

@neilweinstock
Copy link
Contributor

@neilweinstock
Copy link
Contributor

OK, I yield. Let's have a primary and secondary preference, as you suggested. There should be an option on secondary to disable it, if reasonably possible.

I am thinking that slash-separating the two values might be better than putting the second one in parens. What do you think?

@SiboVG
Copy link
Member Author

SiboVG commented May 25, 2023

OK, I yield. Let's have a primary and secondary preference, as you suggested. There should be an option on secondary to disable it, if reasonably possible.

One possibility is to disable the secondary unit when it is the same as the primary unit, but that is probably not straight-forward to users. Probably better to add a checkbox under the secondary stability unit preference. I would actually implement both; hide the secondary unit when the checkbox is unchecked in the preferences or the primary and secondary unit are equal.

I am thinking that slash-separating the two values might be better than putting the second one in parens. What do you think?

When using the secondary preference idea, we should indeed use a slash.

@neilweinstock
Copy link
Contributor

One possibility is to disable the secondary unit when it is the same as the primary unit, but that is probably not straight-forward to users. Probably better to add a checkbox under the secondary stability unit preference. I would actually implement both; hide the secondary unit when the checkbox is unchecked in the preferences or the primary and secondary unit are equal.

That sounds perfect.

@hcraigmiller
Copy link
Collaborator

Functions as described, with no anomalous behavior found.

OR Build: 1761
Windows 11 Pro; Version 22H2; Build 22621.1265; Windows Feature Experience Pack 1000.22638.1000.0
Java version "11.0.18" 2023-01-17 LTS; Java(TM) SE Runtime Environment 18.9 (build 11.0.18+9-LTS-195)

@SiboVG SiboVG changed the title [#2079] Display percentage length unit in stability info [#2079] Display secondary stability unit May 29, 2023
@SiboVG
Copy link
Member Author

SiboVG commented May 29, 2023

Secondary stability unit can now be chosen in the preferences + can be disabled.

Screen.Recording.2023-05-29.at.15.24.52.mp4

@neilweinstock
Copy link
Contributor

Looks great.

@hcraigmiller
Copy link
Collaborator

Very nice.
Functions as expected, no anomalous behavior found.

OR Build: 1765
Windows 11 Pro; Version 22H2; Build 22621.1265; Windows Feature Experience Pack 1000.22638.1000.0
Java version "11.0.18" 2023-01-17 LTS; Java(TM) SE Runtime Environment 18.9 (build 11.0.18+9-LTS-195)

@SiboVG SiboVG merged commit 2fc2c64 into openrocket:unstable May 29, 2023
@SiboVG SiboVG deleted the issue-2079 branch May 29, 2023 20:42
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.

[Feature Request] Add option to show stability in cals and as a percentage

3 participants