Replace certain labels with clearer values

[jasonhenriquez]

Replace certain labels with clearer values

Pull Request Type

• Bugfix
• Feature Implementation
• Documentation
• Other

Related issue

N/A; although, general reports of confusion with these labels in comments, issues, and the Matrix channel

Description

en-US label changes:

• Next Video Interval -> Autoplay Countdown Timer
• Enter Fullscreen on Display Rotate -> Enter Fullscreen On Rotating the Device
• Enable Screenshot -> Enable Video Screenshot
• Hide Unsubscribe Button -> Hide Subscribe Button*
• Play Next Video -> Autoplay (Recommended Channels and Playlists)*
• Autoplay Playlists -> Autoplay (Playlists Only)*
• Autoplay Videos -> Start Videos Automatically*
• Enable Theatre Mode by Default -> Enable Theater Mode by Default*

• variable naming updated as well

Behavioral change:

• The Playlist Autoplay toggle in Player Settings is now disabled when General Autoplay is enabled (to more clearly represent that it is a subset of its behavior)

New dev utility:

• Creates the aliasToOriginal object in settings.js to allow for easy, safe, & reverse-compatible modification of older existing settings variable names

Testing

• Before running the PR locally, set the values of the variables marked by the asterisk above to ones different from their default. Confirm those selections are still preserved while running this PR.
• While running this PR locally, change the values of the variables marked by the asterisk above to new ones again. Run development from your local and confirm that those selections are still preserved.
• In Player Settings, validate that Autoplay (Playlists Only) is disabled as a choice when Autoplay (Recommended Channels and Playlists) is enabled.
• Confirm no errors appear on the affected routes (Watch and Settings)

Desktop

• OS: OpenSUSE Tumbleweed
• OS Version: 2023xxxx
• FreeTube version: 0.19.1

Additional context

I kept the original keys of en-US.yaml intact to prevent any disruption to non-English language users.

[jasonhenriquez]

@absidue Your suggestion has been implemented. Only last question is if we want to comment this line out for one release to allow new release toe-dippers from being disrupted.

[absidue]

I like this more but it's still not quite what I had in mind.

My idea was that the fixing of the settings would happen inside the datastore base handler settings class (src/datastores/handlers/base.js) in the relevant query functions e.g. find(), _findAppReadyRelatedSettings(), etc (you'll have to check which ones are actually affected). That way it would be the only place that would have to have any code for renamed settings, the rest of FreeTube like the store, would not have to care that settings were ever renamed, as it's handled in one place at the lowest possible level.

[jasonhenriquez]

Want to make sure I understand what you mean - have application logic in src/datastores/handlers/base.js that queries for a specific set of settings before find(), uses each one's value to supplant their new equivalent's, and then deletes them? We don't have any other app logic there, so I worry that might be a bit out of place if I'm reading you right.

[jasonhenriquez]

@absidue Apologies, just boosting this q in case you didn't see.

[jasonhenriquez]

@absidue Apologies to boost this once again, but still awaiting your response on this last part. We don't actually have any application logic in that file, and I don't think it's confusing in the code currently which code corresponds to the renaming logic (just one for-block and object). It also lets us reuse a similar paradigm back-to-back (see the code itself). I am willing to refactor this if there is a strong enough pro to moving it (+ the time commitment), but I'm having trouble seeing that at the moment.

[absidue]

Personally I see these changes as database migrations, which I would prefer to keep as close to the actual databases as possible. It also keeps the code a lot simpler, as only place that has to care about the settings having another name the Settings.find() method in the src/datastores/handlers/base.js file, instead of your current approach that spreads it across many files and also requires IPC calls to make the changes, instead of being able to directly modify the database.

You wouldn't need any extra find calls. just do the normal find all call on the database, modify it if necessary, make the database modifications if necessary and then return the response, that can all happen inside the Settings.find() method in the src/datastores/handlers/base.js file.

[efb4f5ff-1298-471a-8973-3d47447115dc]

@jasonhenriquez could u maybe make changes to incorporate #1682 to your PR?

[jasonhenriquez]

@jasonhenriquez could u maybe make changes to incorporate #1682 to your PR?

We discussed this over the Matrix channel, but just so others can see, this will not be included in this PR due to its effect on other ongoing efforts.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[jasonhenriquez]

I'm not capable of solving this one in a reasonable amount of time. Someone else please take this up.