Bugzilla – Bug 2647
Multiple selection (checkbox, radio button) player UI should be consistent on both SN and SS
Last modified: 2008-09-15 14:39:24 UTC
Instead, just show the box getting checked off. The flash makes it hard to see what's happening.
note: this change should be accompanied by a change on the wiki: http://wiki.slimdevices.com/index.cgi?UserInterfaceGuidelines it states: "Pressing RIGHT when one of these is displayed should show briefly: "Saving your selection..." and then display the updated setting."
Ah, good point. I updated the wiki.
So is this a change in one of the INPUT.* modules?
i think it is littered over a few places. It would be a problem with the overlay functions, which are set outside of INPUT.*. Many of the settings module don't even use the checkbox UI yet
Dean - which menu were you seeing this happen on?
nothing in the code now seems to be using the checkbox with showBriefly. many toggle items arent' using the checkbox overlay.
So is this still a valid bug?
Subject: Re: When ticking off checkboxes, don't showBriefly yes.
ok, so what is needed to fix this? no checkBoxOverlay callers in slimserver are following up with showBriefly, so is this just a squeezeNetwork bug? If this is about making more use of the checkboxOverlay, the summary should change,and we should gather a list of where it needs to be added.
Subject: Re: When ticking off checkboxes, don't showBriefly > ok, so what is needed to fix this? no checkBoxOverlay callers in > slimserver > are following up with showBriefly, so is this just a squeezeNetwork > bug? Part of it is cleaning up the calls to showBriefly. The other part is making our UI consistent on both SS and SN to use checkboxes and radio buttons consistently across the UI. > If this is about making more use of the checkboxOverlay, the > summary should > change,and we should gather a list of where it needs to be added. Good point.
The checkbox UI used in SN should be used more generally in SlimServer. Both sides should not use showBriefly when a change is saved. It would be great if a new button mode was available that handled the checkbox UI appropriately so the caller didn't need to worry about showBriefly, etc... (And if I change my mind and decide that we DO need this confirmation, we can add it to the mode and the callers will get it for free.)
INPUT.Choice seems a good candidate. It is intended for selection and action on one item in a list, where descending is not usually required. It is INPUT.Choice that's being used for list selection in SN is it not? ie, for the natural sounds selection in the alarm? INPUT.List could also be done, but it is more of a navigation mode, where descending further is the more likely intent. Annoyingly, these modes are just so similar in intent yet such different code. I wish I had more time as these really do seem like something that could be merged and both behaviours modified via the params given.
Created attachment 1389 [details] modifications to INPUT.Choice this option triggers on the lack of an overlay pref. If the user provides a 'pref' name, instead of any overlay, then we'll assume a non-descending selection list. this can automatically use the checkbox overlay, with a client update on every lines function call (could instead go in the changePos). This means the caller doesn't even have to worry about updating the display when the checkbox changes, and the use of INPUT.Choice in this function is a very simplified set of params. onRight and overlayRef are not required. We can even default so that onPlay, onRight, and onAdd are all one function callback.
So I tried your patch, and removed the overlayRef from the Digital Input selection, and added a 'pref' line. But I get this when going into the Setting: 2006-08-01 12:21:39.7904 Deep recursion on subroutine "Slim::Buttons::Input::Choice::lines" at /Users/dsully/dev/slim/current/Slim/Display/Display.pm line 147. 2006-08-01 12:21:39.7909 Deep recursion on subroutine "Slim::Player::Player::update" at /Users/dsully/dev/slim/current/Slim/Buttons/Input/Choice.pm line 413. 2006-08-01 12:21:39.7910 Deep recursion on subroutine "Slim::Display::Display::update" at /Users/dsully/dev/slim/current/Slim/Player/Player.pm line 221.
yeah...update can't go in there automatically after all. If we are sure it should be an automatic update, then if the 'pref' exists, we can also call a $client->update following the callback if there is no error: if ($@) { errorMsg("INPUT.Choice: Couldn't run callback: [$callbackName] : $@\n"); } elsif (getParam('pref')) { $client->update; } We could always update, skipping the 'pref' check, or create a new param to indicate that the mode is checkbox type, expanding in future to perhaps a radio button type as well. It depends on whether this ui would ever be useful for anything other than prefs.
Created attachment 1392 [details] update on callback, when 'pref' exists. patch for the above
settings in slimserver updated at change 8906
Dean approved. Thanks for the hard work, kdf. Closing.
has SN got the related ports?
Yes, there is a branch of SN for Transporter support that is current with trunk.