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

Update up-next text when auto play next episode is disabled #5666

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

ConnorS1110
Copy link
Contributor

Changes
When Play next episode automatically is disabled, but the up-next popup is enabled, it still shows a countdown timer, which is misleading because it will not auto play the next episode. It now checks user configuration settings to see if you have the popup enabled and have EnableNextEpisodeAutoPlay set to false, which will now just show Next Episode/Video instead of also including the countdown timer.

Issues
Fixes #5643

@ConnorS1110 ConnorS1110 requested a review from a team as a code owner June 6, 2024 02:05
@ConnorS1110 ConnorS1110 force-pushed the update-up-next-dialog-text branch from 1ee5c66 to 07ce87f Compare June 6, 2024 02:07
@thornbill thornbill added enhancement Improve existing functionality or small fixes ui & ux This PR or issue mainly concerns UI & UX labels Jun 10, 2024
@thornbill thornbill added this to the v10.10.0 milestone Jun 10, 2024
src/strings/en-us.json Outdated Show resolved Hide resolved
@ConnorS1110 ConnorS1110 force-pushed the update-up-next-dialog-text branch from 07ce87f to 1b4f5a4 Compare June 10, 2024 19:21
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@ConnorS1110 ConnorS1110 force-pushed the update-up-next-dialog-text branch from 1b4f5a4 to 076f6e5 Compare July 13, 2024 16:30
@thornbill thornbill merged commit e92b9f0 into jellyfin:master Jul 26, 2024
10 checks passed
@854562
Copy link
Contributor

854562 commented Jul 26, 2024

I saw the new strings from this PR pop up on Weblate and noticed that the separate strings for "next episode" and "next video" have been combined into one string with a variable: Next {0} Playing in {1}, in which {0} is either Episode or Video. I believe this creates potential localisation issues.

  1. In some languages, the form of the adjective (next) depends on the grammatical gender of the noun it pertains to (episode, video). For languages in which the translations for "episode" and "video" are of different grammatical genders, this creates a problem.

  2. In English, Title Case is very common for dialogue texts like this, but in many languages (including Dutch, which I am most familiar with), Title Case is Never a Thing and sentence case is always used. Therefore, "episode" and "video" should not start with a capital letter in these languages, which this string change makes impossible.

I realise these points, particularly the second, might come across as nitpicky, but I did want to point them out regardless. In summary, I would suggest reverting the string changes, since as far as I understand, the previous situation worked just fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality or small fixes ui & ux This PR or issue mainly concerns UI & UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Next episode" box language implies the next episode will autoplay even when autoplay is off
3 participants