osc.lua: cycle tracks when there's only one #15186
Conversation
guidocella
force-pushed
the
osc-1-track
branch
from
a576f7a
to
9d96bdd
Compare
guidocella
force-pushed
the
osc-1-track
branch
from
9d96bdd
to
94c89cd
Compare
|
Download the artifacts for this pull request: |
When there is 1 audio or sub track, make left click select or unselect it directly without opening the selector. The script-opt command is ignored in this case. Requested by sfan5.
guidocella
force-pushed
the
osc-1-track
branch
from
94c89cd
to
c66ed39
Compare
|
I see the idea and it make sense, but I'm worried about consistency of the button action. For example user might want to click on the tracks to see what are available and decided to dismiss the selection. This is easily done with left/right click currently. Now if we change it to do something different when there is 1 track user might turn off the track when the intention is to show list. I know that there is a number, but even then user might want to see current track info. I also know there is middle mouse binding, but this is an extra, middle mouse is not that accessible except on external mice. That's said, idea is good, I just want to raise my concern about consistency. "Hidden" conditions like this one makes UI more complex to understand, while it should be simple and accessible. |
|
How about making the icons and the numbers separate buttons? Clicking on the number cycles tracks and clicking on the icon brings up the select menu. I think this makes sense, since clicking on the numbers increases/decreases the track number by one, and clicking on the icon makes sense to perform some non-cycling action. This way, the new feature is accessible, and existing users can still get the old behavior by default by clicking on numbers. |
I understand the risk but think in this case it still provides a simple and helpful behavior.
Too confusing and the click targets are small enough as-is IMO. |
It's the same size as play/pause, prev/next chapter, mute, fullscreen buttons. I don't see how this is a problem. |
| @@ -1866,7 +1866,9 @@ local function osc_init() | |||
| return ("\238\132\134" .. osc_styles.smallButtonsLlabel .. " " .. | |||
| (mp.get_property_native("aid") or "-") .. "/" .. audio_track_count) | |||
| end | |||
| ne.eventresponder["mbtn_left_up"] = command_callback(user_opts.audio_track_mbtn_left_command) | |||
| ne.eventresponder["mbtn_left_up"] = command_callback(audio_track_count == 1 | |||
| and "cycle audio" | |||
There was a problem hiding this comment.
This shouldn't be hardcoded. It defeats the purpose of custom commands when they only work with >2 tracks.
There was a problem hiding this comment.
The previous version checked if the default command was changed and sfan made me change it. ¯\(ツ)/¯
There was a problem hiding this comment.
That depends on what the purpose of the custom command was.
Is it so the user can use his own selection script? cycling would still make sense in that case.
Or so a totally custom action can be defined for the button? (use case?)
Either way the alternative is to change the default to "" and use that to indicate this special behavior.
There was a problem hiding this comment.
The previous version checked if the default command was changed and sfan made me change it. ¯_(ツ)_/¯
You just add this custom handling as a script message, then it can be written as a command.
That depends on what the purpose of the custom command was.
It's for allowing arbitrary commands. It can be used to show custom track information formatting and duration when switching tracks, custom handling depending on the number of tracks, etc.
There was a problem hiding this comment.
We can also just leave it as it is now that right click cycles tracks again. We keep the UI consistent as kasper noted, and if you want to cycle 1 track you can right click. Otherwise I would rather add 2 more script-opts for commands with 1 track than a script message.
There was a problem hiding this comment.
You just add this custom handling as a script message, then it can be written as a command.
This would be proper solution. Having hardcoded conditions/commands beats the purpose of having it customizable. We cannot assume certain use of those buttons, if they are customizable. I already mentioned that about enabled conditions, that currently prevent to customize forward/back buttons.
(but I still think we should keep it simple and consistent)
This is actually clever idea. Not sure if not too complex. But our OSC already has huge track numbers, which is not that common. It would make both select and cycle work together. I'm not 100% sold on this idea yet, but on paper is sounds good. |
|
It should be noted that in the box layout track icons and numbers are much smaller. |
|
OSC doesn't have huge track numbers across all layouts, and you can configure the osc to be much smaller as well. I like this PR and don't like making the icons and numbers their own buttons. |
One case where this would be particularly annoying is when playing music with covert art, you may click the audio track button to see information about the audio track, and mpv will close instead. I had this happen while testing this PR and thought it was a bug. |
When there is 1 audio or sub track, if the command wasn't configured, make left click select or unselect it directly without opening the selector. Unfortunately this is awkward to implement with the command script-opts.
Requested by sfan5.