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

Support change theme in preference panel #549

Merged
merged 4 commits into from
Aug 8, 2019

Conversation

wqz-leo
Copy link
Contributor

@wqz-leo wqz-leo commented Aug 5, 2019

Added change theme pop up button in preferences panel.
Allows user to select preferred theme at runtime.
Save selected theme to UserDefaults.
Defaults to system preferences.

Screenshots

Screen Shot 2019-08-08 at 2 52 09 pm

License

I AGREE TO THE GITUP CONTRIBUTOR LICENSE AGREEMENT

@lucasderraugh
Copy link
Collaborator

We shouldn't be defaulting to a theme one way or the other. In general we should be matching what the system is currently defined as. Now the way I would accept and the way I have seen this work that I think is a good idea is to have 3 options.
Screen Shot 2019-08-05 at 7 58 53 AM
On a separate note, was there something about dark mode that you didn't enjoy? Or do you like it but wanted it while the rest of the system was light?

@wqz-leo
Copy link
Contributor Author

wqz-leo commented Aug 6, 2019

Hi @lucasderraugh ,

I enjoyed the dark mode so much! However, I feel that there might be someone who would like to continue with light mode. At least we should provide an option for them to disable dark mode. IMHO theme preference should be personal but currently there's no option to disable dark mode.

I definitely can add a third option to obey system preference, but I think providing Light and Dark should be good for now. On the other hand, users probably won't change system theme nor GitUp theme very often. Please let me know if you want me to add the third option.

@lucasderraugh
Copy link
Collaborator

lucasderraugh commented Aug 6, 2019

@wqz-leo I like the idea, but I really need the third option to go with system preference (as the default). As it stands in this PR, everyone is going to be defaulted to dark (even if they use light mode), which is not listening to the system at all. There is also the ability coming in Catalina for the mode to dynamically change depending on time of day (and other 3rd party apps do this today).

So, I'd love the changes, but I'm convinced we need to always respect the system by default.

@lucasderraugh lucasderraugh self-requested a review August 6, 2019 03:07
Copy link
Collaborator

@lucasderraugh lucasderraugh left a comment

Choose a reason for hiding this comment

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

Changes needed:

  • Compare menu items by represented object or tag
  • Have 3 options instead of 2 and default to using the system preference by default (System Preference, Dark, Light).

GitUp/Application/AppDelegate.m Outdated Show resolved Hide resolved
Copy link
Collaborator

@lucasderraugh lucasderraugh left a comment

Choose a reason for hiding this comment

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

Fix the baseline alignment and this will be good to go. Thank you so much for doing this!

GitUp/Application/Base.lproj/MainMenu.xib Show resolved Hide resolved
@lucasderraugh lucasderraugh merged commit e1d6572 into git-up:master Aug 8, 2019
@wqz-leo wqz-leo deleted the feature/theme-preference branch August 8, 2019 23:13
simpzan pushed a commit to simpzan/GitUp that referenced this pull request Oct 22, 2020
* support change theme in preference panel

* fix theme preference options

* format code

* make theme popup button and text field baseline aligned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants