Dynamo
Dynamo copied to clipboard
[DNM] Align node previews with number format selected in preferences
Purpose
See slack thread.
Declarations
Check these if you believe they are true
- [ ] The codebase is in a better state after this PR
- [ ] Is documented according to the standards
- [ ] The level of testing this PR includes is appropriate
- [ ] User facing strings, if any, are extracted into
*.resx
files - [ ] All tests pass using the self-service CI.
- [ ] Snapshot of UI changes, if any.
- [ ] Changes to the API follow Semantic Versioning and are documented in the API Changes document.
- [ ] This PR modifies some build requirements and the readme is updated
Release Notes
(FILL ME IN) Brief description of the fix / enhancement. Mandatory section
FYIs
@Amoursol
It looks good @aparajit-pratap - maybe we can add a simple test though using one of the customer graphs that prompted this change?
Let's wait for a decision from @Amoursol, @Jingyi-Wen, and the rest of the team on the next steps for this. For this PR to be given the green light some of the questions that need answering/assumptions made are:
- Are we okay to base the node preview format on the number format chosen in preferences? I don't recall if the number format was targeted for number nodes only or for the overall UI. I leave this decision to UX and others.
- If we're good with point (1) do we want to add a "default" option to the existing number format options that doesn't limit the number of decimal places? This will require more changes to this PR that are simple to add.
- Are we okay with the idea that the number of decimal places (rounding) is only a visual thing and that the original numbers, for example, 0.0199999999999996, will continue to be used in actual computations?
Let's wait for a decision from @Amoursol, @Jingyi-Wen, and the rest of the team on the next steps for this. For this PR to be given the green light some of the questions that need answering/assumptions made are:
- Are we okay to base the node preview format on the number format chosen in preferences? I don't recall if the number format was targeted for number nodes only or for the overall UI. I leave this decision to UX and others.
- If we're good with point (1) do we want to add a "default" option to the existing number format options that doesn't limit the number of decimal places? This will require more changes to this PR that are simple to add.
- Are we okay with the idea that the number of decimal places (rounding) is only a visual thing and that the original numbers, for example, 0.0199999999999996, will continue to be used in actual computations?
@aparajit-pratap thank you for putting this PR in! My answers as below:
- This is a question for @Jingyi-Wen, but it will largely depend on what those settings do today and if they can be conflated.
- We 💯 want to add a default option if we use this, yes.
- Typically, we are trying to index into clarity for all things in Dynamo, so that every single user can understand what is going on, ideally at a glance. The tricky bit with the behavior before this PR was that it was hidden in the UI from the user, and only exposed via nodes, causing confusion. We may need another Preference setting here that enables rounding under a certain threshold for users, as users coming in from the Revit world may not care about infinitesimally small numbers and prefer rounding. Would love to discuss this one with @Jingyi-Wen too.
Let's wait for a decision from @Amoursol, @Jingyi-Wen, and the rest of the team on the next steps for this. For this PR to be given the green light some of the questions that need answering/assumptions made are:
- Are we okay to base the node preview format on the number format chosen in preferences? I don't recall if the number format was targeted for number nodes only or for the overall UI. I leave this decision to UX and others.
- If we're good with point (1) do we want to add a "default" option to the existing number format options that doesn't limit the number of decimal places? This will require more changes to this PR that are simple to add.
- Are we okay with the idea that the number of decimal places (rounding) is only a visual thing and that the original numbers, for example, 0.0199999999999996, will continue to be used in actual computations?
@aparajit-pratap thank you for putting this PR in! My answers as below:
- This is a question for @Jingyi-Wen, but it will largely depend on what those settings do today and if they can be conflated.
- We 💯 want to add a default option if we use this, yes.
- Typically, we are trying to index into clarity for all things in Dynamo, so that every single user can understand what is going on, ideally at a glance. The tricky bit with the behavior before this PR was that it was hidden in the UI from the user, and only exposed via nodes, causing confusion. We may need another Preference setting here that enables rounding under a certain threshold for users, as users coming in from the Revit world may not care about infinitesimally small numbers and prefer rounding. Would love to discuss this one with @Jingyi-Wen too.
My thoughts:
- User would expect the number format they chose in Preference to be universal, so it would be ideal if we can align them all
- I like @Amoursol's idea, we can add a rounding setting
Regarding "We may need another Preference setting here that enables rounding under a certain threshold for users", I believe this is a bigger task that would need addressing at a core level that would affect all numeric results, certainly not in this PR. Until then I suppose we can either wait until those changes happen or first roll out this part where we are only using preferences for display purposes.
@Amoursol @Jingyi-Wen any thoughts about the ☝️ suggestion?
Regarding "We may need another Preference setting here that enables rounding under a certain threshold for users", I believe this is a bigger task that would need addressing at a core level that would affect all numeric results, certainly not in this PR. Until then I suppose we can either wait until those changes happen or first roll out this part where we are only using preferences for display purposes.
@Amoursol @Jingyi-Wen any thoughts about the ☝️ suggestion?
Looks good to me @aparajit-pratap - We can look to further rounding improvement later.
Closing since this is now duplicating https://github.com/DynamoDS/Dynamo/pull/13744