-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
gui: Disable Save button in modals when invalid or no changes #9089
base: main
Are you sure you want to change the base?
gui: Disable Save button in modals when invalid or no changes #9089
Conversation
Currently, the modals have their Save buttons enabled regardless if anything has been modified or the input values are valid or not. This makes it so that the buttons become active only when the user has either edited any of the values and the input values are valid. For this to work in the Advanced Configuration modal, Folders in advancedFolders, Devices in advancedDevices, and Default Folder, Device and Ignore Patterns in advancedDefaults need not use separate forms for each repeated element but rather they must be wrapped into a single, larger form. Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea. I have only glanced at the code so far, hope to take a closer look and do some testing soon.
No problem, take your time 🙂. For the record, this is just a crude method to detect whether the user has edited anything. The proper way would be to also check whether the modified values actually differ from their initial state (see https://stackoverflow.com/questions/40485315/reset-form-as-pristine-if-user-changed-input-values-to-their-initial-value), but I think that this shall be a much more serious undertaking for a different PR. On side note, the GitHub's diff is really not great as it shows seemingly a lot of code changes while in the Advanced Configuration modal it's really just a few form tag modifications plus a lot of code alignment fixes with no actual changes. |
You can hide whitespace only changes, very useful for cases like this ( Change looks sensible to me. |
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Thank you! I've managed to find the button 😅. So much better now! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small indentation fixes required. Maybe teach your editor to fix that?
On a side note, Even My Aunt Crashes the System by invoking C-Home C-Space C-End M-x indent-region RET. But I wouldn't seriously expect anyone to start learning such stuff today... 😉
Seriously, there is a logic error if you change something in a modal, then cancel and open it again, the Save button is still enabled. Probably need to reset the dirty flag on load?
f0aed75
to
cbadebe
Compare
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
cbadebe
to
9f308a3
Compare
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Fixed. I've followed the same logic as already used for |
Even though I understand the idea behind it and do like it overall, from an end-user's perspective it's not too great at this point, in my opinion. 1.) It can be triggered by more than one condition while it's unclear which of them triggers it. When everything works as expected this is probably not an issue, but some odd things happen and you're just scratching your head. This, for example, is also the case for the fav-icon (exclamation-mark)...there's little as frustrating as not knowing why something is the way it is without it being very clear. Even more so if it actually has consequences for what we can or cannot do. And, I think that this was already mentioned, the behaviour overall should be a bit more consistent imo; properly tracking changes. A space -> backspace enables the button and keeps it enabled, that kind of negates half of the point of this feature? |
gui: Disable Save button in modals when invalid or no changes
Currently, the modals have their Save buttons enabled regardless if
anything has been modified
or the input values are valid or not. Thismakes it so that the buttons become active only when the user has
either edited any of the values and the input values are valid.
For this to work in the Advanced Configuration modal, Folders in
advancedFolders, Devices in advancedDevices, and Default Folder, Device
and Ignore Patterns in advancedDefaults need not use separate forms for
each repeated element but rather they must be wrapped into a single,
larger form.
Signed-off-by: Tomasz Wilczyński twilczynski@naver.com