Open RussKie opened 8 years ago
There are certain pages like scripts that you can't cancel.
On Sat, Aug 27, 2016 at 11:06 AM RussKie notifications@github.com wrote:
The purpose/mechanics of these buttons is rather confusing
[image: image] https://cloud.githubusercontent.com/assets/4403806/18028010/5491d476-6cb6-11e6-972a-dcaba1c57bad.png
That's how the buttons work now:
- [OK] - save and close, all configurations reloaded
- [Cancel] - close, all configurations reloaded
- [Apply] - accept all changes made. On some screens configuration changes are saved instantly, can't undo (let's call it [Instant Apply])
- [Discard] - all configurations reloaded All of the buttons are always visible and enabled
However together [Cancel] and [Instant Apply] seem mutually exclusive - if I applied, I can no longer cancel... Or if I want cancelable changes then I can't really instantly apply them.
Perhaps the mechanics should be as follows:
- [Close] - close, if no changes. Changes to [Save and close] if there are pending changes, accept any pending changes, save and close. Save should apply and reload any global configurations, if required (i.e. render the UI in the new language). This button is always visible
- [Cancel] - close and discard all of the changes (so no change at all). This button is conditionally visible. Should confirm with the user.
- [Apply] - accept all pending changes across all settings pages and apply to the current settings copy. This button is conditionally visible
- [Discard] - undo all pending changes across all settings pages and revert to the last settings copy (i.e. to the previous [Apply]ed state). This button is conditionally visible. Should confirm with the user.
I think it can be achieved if the settings form was working with a copy of the settings rather than the original (which I think it is https://github.com/gitextensions/gitextensions/blob/master/GitCommands/Settings/AppSettings.cs#L91) and then accepted or discarded the copy based on the user decision (which it doesn't). I think tracking of the dirty state can be done via SetValue
method https://github.com/gitextensions/gitextensions/blob/master/GitCommands/Settings/RepoDistSettings.cs#L59 and then by comparing the state of the copy to the original. There are also several bugs (IMO) resulted from the current implementation:
- Some configuration may not be reloaded/applied correctly without the app restart, if 'Apply'ed: correct change language -> [OK] - the new language is applied, UI is refreshed incorrect change language -> [Apply] -> [OK] - new language is sort of applied, UI is not refreshed
- Whenever you click 'Cancel' the form is closed and all settings are reloaded, which causes flickering of UI and overall unresponsiveness.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/gitextensions/gitextensions/issues/3311, or mute the thread https://github.com/notifications/unsubscribe-auth/ADdhsWwM8L8lRS0KN6FBp_lhxO-KrZPdks5qkFJbgaJpZM4Jutsp .
Can you write what conditions have to be met in order to make the conditionally visible buttons visible?
There are certain pages like scripts that you can't cancel.
Can't they be run after [OK] clicked?
Now we have "Ok", "Cancel" and "Apply". I think the issue can be closed.
The purpose/mechanics of these buttons is rather confusing
That's how the buttons work now:
However together [Cancel] and [Instant Apply] seem mutually exclusive - if I applied, I can no longer cancel... Or if I want cancelable changes then I can't really instantly apply them.
Perhaps the mechanics should be as follows:
I think it can be achieved if the settings form was working with a copy of the settings rather than the original (which I think it is) and then accepted or discarded the copy based on the user decision (which it doesn't). I think tracking of the dirty state can be done via
SetValue<T>
method and then by comparing the state of the copy to the original.There are also several bugs (IMO) resulted from the current implementation: