GNOME Bugzilla – Bug 473332
Save button should be insensitive until sync service settings are empty
Last modified: 2017-07-31 12:37:41 UTC
To use the syncronization service, I have to fill in 1-3 text fields to configure it, but before I do so, it's pointless to make the Save button sensitive, because pressing it results only an error message.
Probably a good idea, but it might constitute a break of the UI freeze that's currently in effect, so we may have to wait until 0.10.0.
Sandy, I'm pretty sure we could fix this without any UI breaks. I'm almost certain that the UI freeze is only to prevent people from adding new UI elements. This one feels like a legitimate bug that needs fixing.
Sounds good, if that's the HIG convention.
Setting the default assignee and QA Contact to "tomboy-maint@gnome.bugs".
Created attachment 173299 [details] [review] Save button should be insensitive until required fields are non-empty I'm not a fan of the Save button honestly. I'm not sure it fits with the Gnome HIG. Usually configuration is saved automatically. I do however understand the need to test the settings are good. In this case, I think we should rename the button to "Test Connection" and we'll save whatever garbage the user enters anyway... but thats another bug. The patch lets the SyncAddin hook up required fields to the listener which will update the Save button. Its the addin author's responsibility to hook up the callback and define the IsSettingsValid property which will check required fields for non-empty.
bump: patch needs review. UI freeze is monday.
Review of attachment 173299 [details] [review]: I like it. Comments are minor but should probably be addressed before push. ::: Tomboy/Synchronization/SyncServiceAddin.cs @@ +28,3 @@ /// </summary> + /// <param name="requiredPrefChanged">Delegate to be called when a required preference is changed</param> + public abstract Gtk.Widget CreatePreferencesControl (EventHandler requiredPrefChanged); Normally we'd put an event on the widget for this instead of passing in an EventHandler. Your call, but I think it would be more consistent to add a PreferenceChanged event or something? I guess that would require a base widget class or something? No code open so I may not be the best judge right now. @@ +54,3 @@ + /// (Required setings are non-empty) + /// </summary> + public virtual bool IsSettingsValid AreSettingsValid Mr. Grammar Man
(In reply to comment #7) > Review of attachment 173299 [details] [review]: > > I like it. Comments are minor but should probably be addressed before push. > > ::: Tomboy/Synchronization/SyncServiceAddin.cs > @@ +28,3 @@ > /// </summary> > + /// <param name="requiredPrefChanged">Delegate to be called when a > required preference is changed</param> > + public abstract Gtk.Widget CreatePreferencesControl (EventHandler > requiredPrefChanged); > > Normally we'd put an event on the widget for this instead of passing in an > EventHandler. Your call, but I think it would be more consistent to add a > PreferenceChanged event or something? I guess that would require a base widget > class or something? No code open so I may not be the best judge right now. You're right... that was dumb on my part. The event handler is to be attached to a gtk widget which would be different per Addin. I was thinking this would be the only way to hookup the event because that extra layer, but in fact the right way would be add an event to the Addin, and the addin would add its own event handler to the gtk widget which would fire the addin's event. Since the UI freeze is Monday, I'm going to push this anyway but we'll leave the bug open to fix the plumbing. On a side note I'm not sure why I labeled this bug a UI change. > @@ +54,3 @@ > + /// (Required setings are non-empty) > + /// </summary> > + public virtual bool IsSettingsValid > > AreSettingsValid Mr. Grammar Man I prefer the consistent "Is" despite grammar but I'll assume it's covered in a coding style doc and change it.
Comment on attachment 173299 [details] [review] Save button should be insensitive until required fields are non-empty Committed as 6135f29b23527acfb31a6f9f066d411a4cbe4241 This will be included in Tomboy 1.7.4 Thanks for reporting.
This breaks WebSync if a downstream provides a default server url. If the serverEntry doesn't get changed, and we have a successful auth, the save button stays insensitive. Ideally this should make the save button sensitive when Auth.IsAccessToken becomes true.
The Tomboy team has moved from GNOME Bugzilla to GitHub for bug reports and feature requests: https://github.com/tomboy-notes/tomboy/issues/ Closing this report as NOTGNOME as part of Bugzilla Housekeeping (bug 781054) to keep tasks in one place. Please feel free to transfer this task to GitHub if this task is still valid in a recent Tomboy version. We are sorry for the inconvenience.