Add variable editing#581
Conversation
src/CalcViewModel/Common/Utils.h
Outdated
| t get() { return m_##n; }\ | ||
| void set(t value) {\ | ||
| m_##n = value;\ | ||
| RaisePropertyChanged(L#n);\ |
There was a problem hiding this comment.
What are we trying to fix with it this macro? I don't think it's a good practice to use PropertyChanged to launch events when nothing changed, better to create a specific event ("Refresh"/"UserInteracted") for example.
There was a problem hiding this comment.
I added this in because it handles a lot of work that I would need to do manually otherwise when binding a TextBox Text property to this property. For example, if a user types invalid input, this will let me set the Value to the previous Value and still fire the event, causing the binding to update, resetting the double in the textbox to the previous value with correct formatting.
I could just manually set the TextBox text in SubmitTextbox instead, but this would require extra work to also format the double -> string conversion to remove trailing zeroes. I noticed you added a utility in one of your recent PRs that lets us remove trailing zeroes very easily from a string. I added a comment to go ahead and remove this work around and use your util function once we re-merge with master and pick up that change.
| { | ||
| auto variableViewModel = static_cast<VariableViewModel^>(sender->DataContext); | ||
|
|
||
| if (sender->Name == "Value") |
There was a problem hiding this comment.
to be sure to not break this function if the XAML is modified:
if(sender == Value)
{
...
}`
The names of these TextBox should probably be changed to remove some confusions and conflicts, I don't expect this->Value to be a TextBox for example:
proposal: ValueTextBox, MinTextBox, MaxTextBox, StepTextBox
Description of the changes:
As part of #338, this PR adds the UI and backend for editing variables that exist for each equation. Variables can be modified with a slider and textbox. Min/Max/Step can be modified by selecting the gear icon for each variable (temporary icon).
The entry point to the UI is temporary as that is still being finalized.
How changes were validated:
Manual testing