View Issue Details

IDProjectCategoryView StatusLast Update
0020620MMW 5Main Panelpublic2024-03-25 13:00
Reporterzvezdan Assigned To 
PrioritynoneSeverityminorReproducibilityalways
Status closedResolutionfixed 
Fixed in Version5.1 
Summary0020620: "Play to" button added to the Player should not have checkable class.
Description"Play to" button added to the Player should not have checkable class. It doesn't have on/off state and should not be skinned as switchable button, since it is in fact a menu button.
Tagstodoc-dev
Fixed in build3002

Activities

michal

2024-02-27 16:33

developer   ~0074384

It does have checked state. Checked state = playback to external player.

zvezdan

2024-02-27 16:38

updater   ~0074385

Last edited: 2024-02-27 16:43

Yes, I know that it has checked state, but as I said, it should not have it. By click on that button you don't toggle anything. That is the point of checked state - to toggle something, right? Instead, it just shows a dropdown menu, which could be closed without choosing anything from it.

If we follow the existing logic, than the "Play to" menu item from the Player menu should also have checkable class, instead of its submenu items.

jiri

2024-02-27 16:48

administrator   ~0074386

Its functionality is similar to Shuffle, Repeat and other buttons around. I.e. can be Off (playing to internal player) or On (playing to another player). So, imho, it really has a 'checked' state.

The only discrepancy I see, is that when 'On', it should not only change its color, but also change to a slightly different icon 'Cast connected': https://mui.com/material-ui/material-icons/?query=cast&selected=CastConnected

zvezdan

2024-02-27 16:55

updater   ~0074387

It seems that we don't understand each other. Here is the screencast and please tell me how is its functionality the same as the Repeat / Shuffle.

jiri

2024-02-28 13:10

administrator   ~0074397

I see, when you don't have a Chromecast device, it might look like there isn't any checked state. It's there only when you have at least one device, see the attached screenshot.

zvezdan

2024-02-28 14:35

updater   ~0074398

It doesn't matter how many items are in that menu, it could be 1 or 100. That doesn't change the fact that that button is a menu button. Click on that button doesn't (and should not) toggle anything, i.e. click on that button doesn't change checked state. The purpose of menu buttons is to show menus, not to toggle something. You are obviously trying to merge two different functions into one button, but this is not usual and intuitive. There is no standard Windows application that use the same button as a menu button and a toggle button.

If you want to have two functions, than you should have two buttons. Just like you have the toolbar menu button for views (down arrow button) and on its left side a toggle button that, you know, toggles something.

Here is the explanation by Microsoft how buttons should look and behave in Windows applications, particularly a toggle split button:
https://learn.microsoft.com/en-us/windows/apps/design/controls/buttons

An excerpt from that page:
"A ToggleSplitButton control has two parts that can be invoked separately. One part behaves like a toggle button that can be on or off. The other part invokes a flyout that contains additional options that the user can choose from.

A toggle split button is typically used to enable or disable a feature when the feature has multiple options that the user can choose from. For example, in a document editor, it could be used to turn lists on or off, while the drop down is used to choose the style of the list."

jiri

2024-02-28 15:33

administrator   ~0074399

While you are possibly right that it is isn't exactly to MS guidelines, this is how Cast button works pretty much in any app. For example desktop Spotify works this way, to name one. So, seems to be fine to keep as is.

zvezdan

2024-02-28 16:16

updater   ~0074400

I don't have Spotify to check it, could you please tell me does click on that Cast button shows a menu or toggle something or both?

zvezdan

2024-02-29 07:16

updater   ~0074437

The thing is that I have a skin which treats toggle buttons differently than menu buttons, and this single button messes with my concept. This is particularly problematic since my skin works with Windows contrast themes that have reduced set of colors and similar design elements that are available with non-contrast themes.

By the way, here are the screenshots how the similar buttons with connectivity look in the Windows tray popup: the left part of buttons is a toggle button and the right part allows choosing a connection. The icon on the right part is a right arrow, instead of a down arrow that is recommended for ToggleSplitButton control, because click on it doesn't show a dropdown menu, but it switches to another page.

jiri

2024-02-29 09:29

administrator   ~0074438

This is how it works in Emby, also the pop-up. I'd say that it's a de-facto standard for Casting to use such a pop-up. So, I don't see how we'd change it for our skins.

That said, if it interferes with your design goals, I wonder whether it couldn't be solved by some tweaks in your skin. It could be either some css modifications of this particular button (applied to 'div[data-id="player-playto"]' css selector) or a more complex solution using some .js code that would really render two-button implementation.
image.png (11,720 bytes)   
image.png (11,720 bytes)   

zvezdan

2024-02-29 11:43

updater   ~0074439

I understand that you want a click on the button to show a list of devices. That is a purpose of menu buttons. What I don't understand is why it should have checkable class. You didn't answer me does a click on the Cast button in the mentioned programs toggle something or not. If you take a look at the de-facto standard between media players, which is Media Player installed on every Windows computer, you wouldn't see that option as checkable (well, it is not even on the main window, but in its menu).

If that button in your program doesn't function as a toggle button, but you just want to show different connected states for the Cast option, then you don't need checkable class, but you could simply change its icon when the connected state is changed, as michal suggested. The program already has the toggle buttons that show different icons for different states, like the left Views toolbar button.

I don't have any device for connecting to test it, but I am wondering what is happening when you have established connection, and then the connection is lost because of some reason, e.g. disconnected network. Does that button change its state in such case? It is not toggled by user in that case, but it should be displayed differently.

You know, it is quite funny that your program has toggle buttons that don't have checkable class (the button to show/hide playing list in mini player (0020644), the left Views toolbar button for switching view, the toolbar button to open/close Search field, the buttons to show/hide Left and Right panel and maybe some more), but then you have this menu button that needs checkable class for some reason.

Since you are mentioning changes in your skins, it is also funny that your own skins supplied with the program don't have that button with assigned checkable class (unlike other chackable buttons in player that have that class assigned in the player.html file). The program is adding that class during runtime for some reason unknown to me.

Well, never mind. If you are fine with the current state, just forget about this and close this issue.

jiri

2024-03-01 11:36

administrator   ~0074463

Yes, the button does change its state in all the casting apps I know -- when you cast, it's different.

We use the 'checked' to mark that it can has different states (call it checked/unchecked or casting/not casting, it doesn't matter).

Leaving this open for Michal though, so that he can also show the 'Casting icon'

zvezdan

2024-03-01 12:25

updater   ~0074465

Well, I didn't ask if that button in other apps changes its state (icon) when it is casting. I asked if that button is checkable by user, i.e. if click on it toggles something.

If you just want an indication about casting state, than you don't need checkable class, but you should use some data attribute, e.g. data-casting = true. And you should use another icon when it is casting, not different color.

But as I said, if you are fine with the current solution, I am fine too. Please close the issue.

michal

2024-03-01 13:47

developer   ~0074466

Last edited: 2024-03-01 13:48

Fixed in build 3002. Checkable class is now set in HTML, not forced in JS, so skins don't need to use it, default skins have it so it works as before for them. Cast button now receives attribute "data-connected", which can be used for styling connected state and cast icon.

zvezdan

2024-03-02 06:29

updater   ~0074471

Thanks!

peke

2024-03-25 13:00

developer   ~0074800

Verified 3007