Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX improvements to the “Jump to…” command #4084

Merged

Conversation

sabberworm
Copy link
Contributor

@sabberworm sabberworm commented Nov 17, 2022


This PR adds small UX improvements to the PlaybackJump to… command:

  1. Allows inputting sub-second precision timecodes to jump to
  2. Displays the current video time code as the initial value
  3. Has the added benefit of making the current time code easy to copy to the clipboard

Initially I had wanted a quick way to copy the current time code. I was already starting to implement a dedicated command for that when I realized the same could be accomplished more synergistically by beefing up the existing “Jump to…” command.

Will also fix #4547

@@ -124,7 +124,7 @@ extension MainMenuActionHandler {
}

@objc func menuJumpTo(_ sender: NSMenuItem) {
Utility.quickPromptPanel("jump_to") { input in
Utility.quickPromptPanel("jump_to", inputValue: self.player.info.videoPosition?.stringRepresentationWithPrecision(3)) { input in
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 sub-second digits is the highest precision selectable in the UI (and commonly used in video editing). I think it’s a good default. Alternatively, we could use the precision the user has previously chosen to be displayed in the player controls.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 digits is good IMO

iina/VideoTime.swift Outdated Show resolved Hide resolved
} else {
return nil
}
let split = Array(format.split(separator: ":").map { String($0) }.reversed())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the new regex builder would be cool to use for parsing but I gather it’s only available with newer versions of swift.

@sabberworm sabberworm changed the title feat(ui): allow quick prompt panel to have initial value UX improvements to the Playback → Jump to… command Nov 17, 2022
@sabberworm sabberworm changed the title UX improvements to the Playback → Jump to… command UX improvements to the “Jump to…” command Nov 17, 2022
@sabberworm
Copy link
Contributor Author

Is there anything I can do to improve the chances of this getting merged? This feature would have been helpful to me numerous times in the past year.

@svobs
Copy link
Contributor

svobs commented Oct 6, 2023

I pulled and tested this fix, and it looks good to me. Unfortunately I can't do anything to get it merged, so 🤞

iina/VideoTime.swift Outdated Show resolved Hide resolved
iina/VideoTime.swift Outdated Show resolved Hide resolved
@sabberworm sabberworm force-pushed the feature/ux-improvements-for-time-jump branch 2 times, most recently from 534ecba to 4514d99 Compare January 3, 2024 13:47
@uiryuu
Copy link
Member

uiryuu commented Jan 5, 2024

Is there anything I can do to improve the chances of this getting merged? This feature would have been helpful to me numerous times in the past year.

@sabberworm Thanks for your contribution. Noted this PR in the backlog. Will merge this before the next beta version.

@sabberworm sabberworm force-pushed the feature/ux-improvements-for-time-jump branch from 4514d99 to 74ccc1c Compare January 5, 2024 13:40
iina/VideoTime.swift Outdated Show resolved Hide resolved
@@ -124,7 +124,7 @@ extension MainMenuActionHandler {
}

@objc func menuJumpTo(_ sender: NSMenuItem) {
Utility.quickPromptPanel("jump_to") { input in
Utility.quickPromptPanel("jump_to", inputValue: self.player.info.videoPosition?.stringRepresentationWithPrecision(3)) { input in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 digits is good IMO

Co-authored-by: Yuze Jiang <i@uiryuu.moe>
Copy link
Member

@uiryuu uiryuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@uiryuu uiryuu merged commit 38b8501 into iina:develop Jan 8, 2024
1 check passed
@sabberworm sabberworm deleted the feature/ux-improvements-for-time-jump branch January 9, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Playback" > "Jump To..." should allow seconds-only input
4 participants