-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[WIP] Subtitles Toggles #3053
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
Draft
WowbaggersLiquidLunch
wants to merge
10
commits into
iina:develop
Choose a base branch
from
WowbaggersLiquidLunch:subtitle-toggle
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
[WIP] Subtitles Toggles #3053
WowbaggersLiquidLunch
wants to merge
10
commits into
iina:develop
from
WowbaggersLiquidLunch:subtitle-toggle
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a very early version of the toggle. It sets subs to None when enabled, restores the previously selected tracks when disabled. Currently mapped as a default IINA command to the "Ctrl+v" shortcut and to the menu item "Disable Subtitles" under the Subtitles menu. It will update it's status automatically to On if the user manually sets both sub tracks to None. In that case, when the toggle is activated, it will set --sid=1 and --secondary-sid=0 by default. It will also automatically resets to Off whenever the user manually activates a subtitle track.
At the same time rename it from `disableSubtitles` to `toggleSubtitlesState`. This commit also includes some non-functional changes automatically generated by Interface Builder.
Since `V` is mpv's subtitle visibility toggle, it's probably better to keep the key bindings as separate as possible. The plan is to have both visibility and state toggles in iina, and have them be user-configurable in preferences. The user can select which subtitle toggle features are enabled: neither, one, or both. The default setting is having the state toggle enabled and visibility toggle disabled.
Rename `subDisabled` to `subtitlesAreDisabled`, then change it to a computed property that equals to the negation of a new stored property `subtitlesAreEnabled`. The renaming brings it in line with Swift API guidelines. Favouring the positive version avoid double-negatives and make reasoning easier. Consolidate sparsely located subtitles state-toggling functions into property observers for the subtitles state variable `subtitlesAreEnabled` and subtitles' track indices. This approach lowers coupling among different components. This commit could've been separated into smaller commits, but Xcode has its limit in picking code snippet for staging.
@unique-username313 Can I either merge or cherry-pick relevant commits from your |
@WowbaggersLiquidLunch Of course you can, but keep in mind that I haven't rebased that branch onto develop for some time now so you might have to deal with some conflicts. |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a draft pull request, wherein not all features are fully implemented yet. In addition to taking advantage of the CI, it acts as a reference point for progress-tracking, and allows everyone an early glimpse of the code itself. The planned features are as follows:
None
tracks.S
key.V
key.slaps roof of pull request this bad boy can fit so much feature enhancement