Bug 2647 - Multiple selection (checkbox, radio button) player UI should be consistent on both SN and SS
: Multiple selection (checkbox, radio button) player UI should be consistent on...
Status: RESOLVED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Player UI
: 6.2.1
: Macintosh Other
: P2 normal (vote)
: ---
Assigned To: Dan Sully
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-28 15:53 UTC by Blackketter Dean
Modified: 2008-09-15 14:39 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
modifications to INPUT.Choice (794 bytes, patch)
2006-07-31 14:40 UTC, KDF
Details | Diff
update on callback, when 'pref' exists. (819 bytes, patch)
2006-08-01 13:07 UTC, KDF
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Blackketter Dean 2005-11-28 15:53:42 UTC
Instead, just show the box getting checked off.  The flash makes it hard to see what's happening.
Comment 1 KDF 2006-07-04 14:04:26 UTC
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."
Comment 2 Blackketter Dean 2006-07-07 15:11:48 UTC
Ah, good point.  I updated the wiki.
Comment 3 Dan Sully 2006-07-23 18:06:48 UTC
So is this a change in one of the INPUT.* modules?
Comment 4 KDF 2006-07-23 18:43:09 UTC
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
Comment 5 Dan Sully 2006-07-25 17:52:53 UTC
Dean - which menu were you seeing this happen on?
Comment 6 KDF 2006-07-25 21:09:29 UTC
nothing in the code now seems to be using the checkbox with showBriefly. 
many toggle items arent' using the checkbox overlay.
Comment 7 Dan Sully 2006-07-26 16:33:55 UTC
So is this still a valid bug?
Comment 8 Blackketter Dean 2006-07-26 16:39:08 UTC
Subject: Re:  When ticking off checkboxes, don't showBriefly

yes.

Comment 9 KDF 2006-07-28 13:15:44 UTC
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.
Comment 10 Blackketter Dean 2006-07-28 13:20:04 UTC
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.

Comment 11 Blackketter Dean 2006-07-28 13:25:33 UTC
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.)

Comment 12 KDF 2006-07-28 15:16:02 UTC
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.
Comment 13 KDF 2006-07-31 14:40:37 UTC
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.
Comment 14 Dan Sully 2006-08-01 12:22:47 UTC
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.
Comment 15 KDF 2006-08-01 12:46:47 UTC
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.
Comment 16 KDF 2006-08-01 13:07:35 UTC
Created attachment 1392 [details]
update on callback, when 'pref' exists.

patch for the above
Comment 17 KDF 2006-08-10 00:25:01 UTC
settings in slimserver updated at change 8906
Comment 18 Dan Sully 2006-08-16 14:58:53 UTC
Dean approved.

Thanks for the hard work, kdf.

Closing.
Comment 19 KDF 2006-08-16 18:33:26 UTC
has SN got the related ports?
Comment 20 Andy Grundman 2006-08-16 19:14:34 UTC
Yes, there is a branch of SN for Transporter support that is current with trunk.