Bugzilla – Bug 4556
Improved settings API
Last modified: 2009-09-08 09:21:27 UTC
As discussed briefly with Dan: I think it seems a little broken have two separate settings api's, Slim/Buttons/Settings.pm vs Slim/Web/*. At the moment to add a setting to both the web and player interfaces you need to code it twice (or i've missed a trick somewhere!). It would be much better if a setting was registered with the slimserver, this would include validation parameters, callbacks, strings, etc. Then code in Slim/Buttons/... and Slim/Web/... could then be used to 'render' these settings to the different interfaces. Not all settings should be available on both the player and web, so an option when registering the setting would include it's visibility ('squeezebox'/'web'/'cli'/etc). The choice of what interface a setting is available on should only be restricted by the visibility, based on UI design issues. Not laziness in writing code! Other advantages would be easy access to all settings via the cli (it's just another setting renderer), consistency between web and player interfaces, and the ability to expose the settings via another interface (for example JSON/OPML/etc).
Part of this has been done a while back by Adrian. Added support for pref validation through command infrastructure (CLI et al.) in change 12245, allowing for asyncronous validation from the web interface through jsonrpc.js
I can see how async validation could be very helpful, but I would hope that most validation can take place client-side via javascript. It would be best to limit the async stuff to things that must be checked server-side, such as checking for the existance of folders, port conflicts, etc. For the best combination of responsiveness, usability and robustness, you want to do validation both on the client and at the server. It's a lot of work, I know. Client side for immediate user feedback before submitting a form. Server side just to be certain that something isn't submitted anyway due to javascript being disabled, or someone doing a POST via a script, or just a simple bug in the client side validation that permits a bad value through.
I'm not sure you tested that new stuff. You say "Client side for immediate user feedback before submitting a form." - that's exactly what it does. And it's fast. Even verifying a file path (a rather expensive job) doesn't take seconds, but only fractions of it. And it's done in the background, while you continue filling out other input fields. Replicating that functionality would be an insane job. Please give it a try and report back if you encounter a problem.
No, I haven't tried it. I'm sure it works quite well. What I'm suggesting is that it would be silly to use it, for example, to verify that an entry is an integer between 1 and 100. That validation can easily be handled in code executed only by the client, without the need to go back to the server. Verifying a file path, on the other hand, is an excellent use for this type of validation, since it can only be done at the server.
Right, that's reasonable. Standard-cases could be done locally only.
Bonus points for forwarding the validators already created in the namespace to the html so that client side can use the existing ranges. Eventually we can also do this with the player ui settings and avoid having to duplicate validation, etc from all different interfaces.
Problem here is we can't use perl on the client side :-). But for the basic validation we should be fine finding some javascript version doing the same. The new "pref validate" command should allow for validation from almost anywhere. Pref::validate() would do the same for eg. settings from the player UI.
I haven't suggested perl on client side. We pass the prefs to the templates, we can pass the ranges. Dan talked about Form::Validator, but that might be far more than is needed. The simplest case would be the integer prefs. How about passing a min and max as part of the prefs hash and pass those to a simple for validator javascript that can pop up an error if the value is out of range. no server involvement until there is a value worth processing :) Happy to help, but not right now.
Most of the simple validators defined in Prefs::Namespace should be relatively easy to reproduce in JS. Except for file & dir. Will try these others.
int ranges and selecting from a valid list/hash would go a long way to reducing what goes back to the server. Since we're already putting ajax to use for the validation, it might even be worth doing the submit posted via ajax as well, eliminating the need to refresh the settings page. If there is an empty div in the page, it can be filled any time with the validation feedback or can be fed to the popup.
Created attachment 2046 [details] ajax page switcher Here is a concept for the settings web pages. The PWDlist is reather redundant in this case, so I've removed it as it's a bother to update via ajax anyway. the header stays with the generic 'Server'- or 'Player Settings' heading, as the pullown shows the current page, as well as provides access at any time to the basic page. Form action gets updated on change to fit the new url and we no longer have a browser frame refresh.
Michael pointed me at this.... My view is that we should consider keeping the validation in one place, but may want to expose lists of options to the client if is capabable of doing e.g. a slider bar to select a value, or drop down menus with options. My reason for suggesting this is to avoid the potential of doing validation in two places and the implementations getting out of sync. Michael's current code works well for me in terms of speed, so I wonder if the current (as committed) solution for pure validation is best? For exposing options to the client, the current code uses hard coded stuff in Slim::Web::Settings. We could consider moving to a scheme where these are built into the preference object rather than web code and then the client could build a preference dialog based on the description string, current value and set of options/limits provided by the perference code. This would mean that e.g. jive would be able to edit options in the same way as the web interface. I think this would work for 90% of the current cases, but it is rather going back in the direction of the old Settings hash!!
Created attachment 2047 [details] ajax submitting the settings page submit the settings page using an ajax request; add animation green for ok prefs, red for invalid settings; display error message at the top of the page
> Here is a concept for the settings web pages. The PWDlist is reather redundant > in this case, so I've removed it as it's a bother to update via ajax anyway. Very nice! With the space freed up in the header of the page (especially in Default skin) I'd like to move the list and the save button to the top, as a kind of toolbar... And we need a concept to tell the browser to reload the page completely, as eg. the language setting doesn't take effect otherwise.
Created attachment 2048 [details] Patches combined++ This patch integrates kdf's and my patches plus adds some feedback to the user during/after background operations.
Kevin - I put the selector box in the page header. This will bring back the "Home" link for Default et al. plus free a few pixels of page height. Shouldn't change anything (visually) in Fishbone.
Created attachment 2049 [details] Another update, cleaning up some code
Change 12265 adds more background processing; needs testing with different skins
Michael Herger thinks this should be closed. If anyone feels otherwise, please reopen.
This bug is being closed since it was resolved for a version which is now released! Please download the new version of SqueezeCenter (formerly SlimServer) at http://www.slimdevices.com/su_downloads.html If you are still seeing this bug, please re-open it and we will consider it for a future release.