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

Enter confirms subtitle track selection #445

Draft
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

pooky-programs
Copy link
Contributor

Implementation of #378
VideoDataSyncDialog now listens for enter key press when subtitle drop downs aren't in focus.
Can be significantly improved if I can figure out how to shift focus from the side panel to the dialog when it opens on click.
Currently it only submits on enter when the dialog is focused after a click, which I understand is not ideal for what the original issue intended.

@killergerbah
Copy link
Owner

Hi @pooky-programs , sorry for taking a long time to look at this. I thought that your changes were still in-progress since the PR is in draft.

Copy link
Owner

@killergerbah killergerbah left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. As mentioned in the comments, I think this change will really be effective when it reduces the clicks necessary for users to select subtitles.

@@ -186,6 +204,8 @@ export default function VideoDataSyncDialog({
return newSelectedSubtitles;
})
}
onFocus={() => setIsDropdownActive(true)}
onBlur={() => setIsDropdownActive(false)}
Copy link
Owner

Choose a reason for hiding this comment

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

As you pointed out, with this implementation it still takes another click for the dialog to respond to the Enter key. I think for this change to be really useful the dialog should close on Enter as long as none of the drop-downs are open. This would reduce the total clicks required to select subtitles.

@pooky-programs
Copy link
Contributor Author

@killergerbah no problem, and yeah this is still WIP! I just haven't had the time to sit down and work on it again, sorry for not telling you sooner. I'll be attempting to fix it after July ends.

killergerbah and others added 25 commits August 4, 2024 10:24
- So that popup dictionary UI is more likely to appear on top
* add default policy if one doesn't exist

* revert speculative fix commit

* add comment linking chrome tests
…e subtitle) (killergerbah#474)

* VideoPlayer.tsx: Don't newline subtitles. (Let yomitan pick up the full sub)

* Videoplayer.tsx: Also wrap text with <p> when dom caching is disabled

* video-player.css: Fix excessive gap on bottom of subtitles (visible with pre-cache subtitle dom + multiple sub track)

* undo inline: messed with splits on subs, whiteSpace made a huge box under the sub box (material ui css stuff)

* videoplayer.tsx: lineheight: normal -> inherit, better spacing look

* prettier
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.

5 participants