Add seek duration by artjomsR · Pull Request #743 · killergerbah/asbplayer

artjomsR · 2025-07-09T15:34:37Z

Implements #287

I've tested firefox and chrome extensions and the local player. I didn't change the android version

Also, when I was generating translations, I couldn't run node ./scripts/loc/merge-loc-from-en.js as I was getting

node:internal/modules/package_json_reader:256
throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
``` so I had to do `yarn add flat`, not sure why

killergerbah · 2025-07-11T13:25:06Z

common/locales/de.json

        "autoPauseAtSubtitleStart": "Am Anfang des Untertitels",
        "autoPausePreference": "Auto-Pause",
        "autoPausePreferenceHelperText": "Aktiviert keine Auto-Pause",
+        "seekDuration": "Amount of seconds to skip fowards / backwards when seeking with respective shortcuts",


This label is pretty long - can we shorten it?

"Seconds to skip forwards/backwards when seeking with respective shortcuts"

That's the shortest I could think of for now

Seconds to skip when seeking forward/backward or
Seek duration (sec) ?

Now what you say it, I'm thinking it'd be better to move these shortcut related settings from MISC into KEYBOARD SHORTCUTS. This way they're gonna be more discoverable by the users and won't need as much helper text because both shortcut and the setting will support each other with context. I'm not sure if there would be any technical challenges with migrating the settings like this?

Yeah I think that would be the ideal solution. Another input field directly under the keyboard shortcuts for seeking left/right.
Similarly, it would make sense for "Always play after invoking 'Seek to beginning of current/previous subtitle'" setting to go underneath the "seek to beginning of current/previous subtitle" shortcut setting.

Ideally there is a solution for both binds. The problem is that we are rendering the keyboard shortcut settings in a for loop over keyBindProperties. First idea that comes to mind is to add a JSX component field (additionalControl or similar) to KeyBindProperties that defaults to null. Then we can just modify keyBindProperties for the two keyboard shortcuts we actually care about, without having to modify the for loop itself with special case logic.

Ok I've pushed the latest commit feaafc7 adding this. I've had to move const {...} = settings init because keyBindProperties now depends on settings.seekDuration

Let me know if you're happy with this approach and I'll tidy up the rest of the PR with the other settings

@artjomsR Sweet, yeah the code looks good. UI-wise I think I would want the new text field have the same width and be aligned with the keyboard binds.

… scrolling

artjomsR · 2025-07-24T15:19:43Z

All done now :) I did a few force pushes to amend my work in progress but didn't change any old commits that you've already seen.

I did some minor corrections to strings for consistency etc
I've changed the UI as you've requested although I thought it was nice that these settings stood out since they're not actually keybinds in the Keybinds section

killergerbah · 2025-07-25T12:25:26Z

@artjomsR Thanks! Really great work and I appreciate you entertaining my suggestions.

Yes I agree that with the alignment being all the same it is difficult to tell the keybinds apart from the additional settings. Maybe there is a solution to this while keeping the alignment - in my mind the alignment needs to be the same.

killergerbah · 2025-07-25T12:29:06Z

common/locales/zh_CN.json

        "autoPauseAtSubtitleEnd": "在字幕末尾",
        "autoPauseAtSubtitleStart": "在字幕开头",
        "autoPausePreference": "自动暂停首选项",
-        "autoPausePreferenceHelperText": "不启用自动暂停。设置启用自动暂停时暂停的首选项。",


Unfortunately we need to keep this key because currently all versions of the extension share the same loc file.

killergerbah · 2025-07-25T12:29:34Z

Hey just one more change and I'll merge ^^
Thanks again so much!

artjomsR · 2025-07-25T12:36:59Z

Yes I agree that with the alignment being all the same it is difficult to tell the keybinds apart from the additional settings. Maybe there is a solution to this while keeping the alignment - in my mind the alignment needs to be the same.

It'll be easy to change later since it's contained in KeyBindRelatedSetting. And most of these settings stand out anyway since they have a different input type

Unfortunately we need to keep this key because currently all versions of the extension share the same loc file.

Just revert the strings in the .JSON files, right? (Even though they won't be used in code). I'm away from my pc today and will try to do it tomorrow, but feel free to revert then if you wish to merge it quicker :)

artjomsR · 2025-07-25T20:48:02Z

Ok done

killergerbah · 2025-07-26T06:48:58Z

Sweeeet!

artjomsR force-pushed the add_seek_duration branch from 877dd40 to d72e981 Compare July 9, 2025 15:42

killergerbah requested changes Jul 11, 2025

View reviewed changes

artjomsR marked this pull request as draft July 15, 2025 16:57

artjomsR added 6 commits July 15, 2025 17:58

Extension: add seek duration

02d018e

Video player support

4ac0fb0

Translations

8888b87

Verify fix + pretty

69b89aa

Update existing 10s strings

e6fe530

Move seekDuration setting to keybinds tab

feaafc7

artjomsR force-pushed the add_seek_duration branch from f9e76c2 to feaafc7 Compare July 15, 2025 16:58

standardise existing subtitles

f7b9c36

artjomsR force-pushed the add_seek_duration branch 3 times, most recently from c0f38bf to d9facde Compare July 24, 2025 13:59

Consistent styling with Keybind field

0a027e4

artjomsR force-pushed the add_seek_duration branch from d9facde to 0a027e4 Compare July 24, 2025 14:01

artjomsR added 4 commits July 24, 2025 15:16

Move alwaysPlayOnSubtitleRepeat

bfdfed3

Move language to the top-ish of Misc settings so it's visible without…

47c0ad7

… scrolling

move alwaysPlayOnSubtitleRepeat

f4d8492

move speedChangeStep

6bc2a3d

artjomsR force-pushed the add_seek_duration branch from 51aae2d to 2402592 Compare July 24, 2025 15:09

move fastForwardModePlaybackRate

81cdb04

artjomsR force-pushed the add_seek_duration branch from 2402592 to 81cdb04 Compare July 24, 2025 15:09

pretty

4bf7c64

artjomsR marked this pull request as ready for review July 24, 2025 15:20

artjomsR requested a review from killergerbah July 24, 2025 15:20

killergerbah added this to the Extension v1.12.0 milestone Jul 25, 2025

killergerbah requested changes Jul 25, 2025

View reviewed changes

revert autoPausePreferenceHelperText

ad5613f

artjomsR force-pushed the add_seek_duration branch from ca05243 to ad5613f Compare July 25, 2025 20:47

killergerbah approved these changes Jul 26, 2025

View reviewed changes

killergerbah merged commit 01c337a into killergerbah:main Jul 26, 2025
1 check passed

Conversation

artjomsR commented Jul 9, 2025

Uh oh!

killergerbah Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

killergerbah Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

artjomsR Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

killergerbah Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

artjomsR Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

killergerbah Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

artjomsR commented Jul 24, 2025

Uh oh!

killergerbah commented Jul 25, 2025

Uh oh!

killergerbah Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

killergerbah commented Jul 25, 2025

Uh oh!

artjomsR commented Jul 25, 2025

Uh oh!

artjomsR commented Jul 25, 2025

Uh oh!

killergerbah commented Jul 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants