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

QOL - Set Theme #113

Merged
merged 6 commits into from
Jul 27, 2021
Merged

QOL - Set Theme #113

merged 6 commits into from
Jul 27, 2021

Conversation

LePips
Copy link
Member

@LePips LePips commented Jul 22, 2021

Allows the user to set the app's appearance based upon "System", "Dark, or "Light".

"System" is default

@LePips LePips requested a review from acvigue as a code owner July 22, 2021 04:49
@LePips LePips changed the title Force app in dark mode QOL - Force app in dark mode Jul 22, 2021
@PangMo5
Copy link
Member

PangMo5 commented Jul 22, 2021

Thank you for the first contribution.

Forcing the app to dark mode does not fit Human interface guideline.
https://developer.apple.com/design/human-interface-guidelines/ios/visual-design/dark-mode

As you said, we need a setting to Dark Mode related.

If you want to update related PR, please update it.
If you don't, please close the PR and post it as an issue.

@LePips
Copy link
Member Author

LePips commented Jul 22, 2021

While forcing the app into dark mode does is initially discouraged by the guidelines, it is acceptable to force a certain mode until we provide the full features that we desire or even running the entire app in a certain mode.
https://developer.apple.com/documentation/uikit/appearance_customization/supporting_dark_mode_in_your_interface/choosing_a_specific_interface_style_for_your_ios_app

Forcing dark mode also parallels Jellyfin's default design of a dark theme.

I am able to do the work to supply the option in the settings as I started some work on it but I intend that to be its own PR/work.

@PangMo5
Copy link
Member

PangMo5 commented Jul 22, 2021

It's literally a recommendation, so it's not a reject to the AppStore, but it's enough to break the UX.

Anyway, I don't think it's a good idea to force dark mode so that users can't set it up.

we can a much better things, like add Appearance setting to Dark Mode related

@LePips
Copy link
Member Author

LePips commented Jul 22, 2021

Apple understands if an app wants to run in a single mode and it does not necessarily break the UX in the meantime that we have it set to dark.

Anyways, I can look at doing the work to set the appearance in settings and update this PR.

@LePips LePips changed the title QOL - Force app in dark mode QOL - Set Theme (WIP) Jul 22, 2021
@acvigue
Copy link
Member

acvigue commented Jul 22, 2021 via email

@LePips
Copy link
Member Author

LePips commented Jul 22, 2021

The OS does, however many users would like to use applications in a mode that differs from their OS. For example, using Discord in dark mode while having my phone use light mode everywhere else.

This is standard design in many apps.

@PangMo5
Copy link
Member

PangMo5 commented Jul 22, 2021

but... the OS literally has a setting for light/dark/auto mode. What do you think @PangMo5

It doesn't exactly fit Apple's HIG. 🤔

Comply with the appearance mode people choose in Settings. If you offer an app-specific appearance mode option, >you create more work for people because they have to adjust more than one setting. Worse, they may think your >app is broken because it doesn't respond to their systemwide appearance choice.

But I think "it's okay" to give the user a choice, if unless forcing a certain Appearance mode.
And The default value should Auto(Naming may vary) that interlocking with system appearance mode.

@LePips
Copy link
Member Author

LePips commented Jul 22, 2021

If you guys would like examples of apps that implement this:

Facebook Messenger, Reddit, Youtube, Twitch, Evernote, TikTok, Twitter

@acvigue
Copy link
Member

acvigue commented Jul 23, 2021

But I think "it's okay" to give the user a choice, if unless forcing a certain Appearance mode.
And The default value should Auto(Naming may vary) that interlocking with system appearance mode.

I agree, a picker in the Settings tab such as "Dark"/"Light"/"System" where System would be default.

@sonarcloud
Copy link

sonarcloud bot commented Jul 26, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@LePips LePips changed the title QOL - Set Theme (WIP) QOL - Set Theme Jul 26, 2021
@acvigue acvigue merged commit 4b92faf into jellyfin:main Jul 27, 2021
@LePips LePips deleted the theme-setting branch October 19, 2021 23:40
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.

None yet

3 participants