killergerbah / asbplayer

Browser-based media player and Chrome extension for subtitle sentence mining
MIT License
426 stars 48 forks source link

Blur shortcuts improvements #451

Open artjomsR opened 1 week ago

artjomsR commented 1 week ago

Is your feature request related to a problem? Please describe. With the current implementation, toggleAsbplayerBlurTrack blur shortcuts persistently enable / disable the blur. This requires 2 key presses to unblur and then blur the subtitle, which is equivalent to using a shortcut to hide & then display the subtitle

To distinguish the shortcuts, I suggest one of the following:

tagging @nekorushi in case you're interested in this discussion :)

nekorushi commented 1 week ago

That's a cool improvement. I am not sure if that should be a separate set of shortcuts or should it replace the current ones (although i feel that your idea would be utilised more often, so it could replace current ones). Personally I won't be able to look into it myself for foreseeable time due to personal responsibilities, but definitely +1 from me.

killergerbah commented 1 week ago

In this scenario, would the settings toggle specify the initial blur state when subtitles load? And the shortcuts only control the blur state during playback?

artjomsR commented 1 week ago

Yes, exactly.

If track blur is off in the settings: blur is permanently off for that track and mouse hover / blur shortcut doesn't change the subtitle appearance

If track blur is on in the settings:

artjomsR commented 1 week ago

I am not sure if that should be a separate set of shortcuts or should it replace the current ones

I think it is fine to replace the existing set of shortcuts. Whoever will use the blur subtitles will be using them regularly as part of their workflow, and I'm not expecting them to use the shortcuts to change the type of the flow that they use

In other words, if they're using blur, I'm expecting them to continue using the blur and not use the shortcuts to periodically toggle that setting on and off

killergerbah commented 1 week ago

I see, makes sense. I agree this would be better.

artjomsR commented 1 week ago

Ok I'll take a look at this then