Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature/Sidebar use theme background color #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

FraDeliro
Copy link

@FraDeliro FraDeliro commented Oct 21, 2018

Purpose
This is a feature to add the option to use theme color for sidebar background color.
Related to #13

Minimum required changes, including:

  • Main.storyboard
  • PreferencesViewController.swift
  • SidebarViewController.swift
  • Preferences.swift

Image after

screenshot 2018-10-28 at 19 39 05

screenshot 2018-10-28 at 19 40 00

@lukakerr
Copy link
Owner

Thanks for this! A few thought on it though before merging it in:

  • The panel in the sidebar at the bottom also should take on the background color. Right now as seen in your screenshot it doesn't and it looks out of place
  • The text and background color of the sidebar items should adapt to the theme background color. If the theme background is dark then the text should be light and the item background should be dark and vice versa

@FraDeliro
Copy link
Author

FraDeliro commented Oct 28, 2018

@lukakerr Thank you for your reply ;) I have updated my PR, at the moment as you can see in the new screenshots, when one item in the sidebar is selected the text is always light but, if not, the textColor changes depending on sidebar background. For the bottom panel instead, the custom view that contains the NSVisualEffectView it's getting now the same sidebar color. I was not sure if you wanted to change the visual effect view color but if there is anything to be improved or modified, let me know :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants