Themes - add title bar class and color options to match sidebar background color for each theme

[paulhirschi]

Since Sublime Text dev build 3127 the option to change the background and foreground color of the window titlebar has been available (only available on OS X 10.10+). In this PR, I've set the titlebar bg color to match the bg color of the sidebar for each theme. See screenshots below for examples.

[paulhirschi]

@saadq Perhaps a more friendly and universally satisfying approach would be to allow a feature like this as a Theme option: "material_theme_title_bar": true rather than forcing users to adopt it.

[saadq]

Firstly, awesome work. Didn't know about this feature, it looks great! Just wanted to confirm – were there no issues in Windows/Linux with having this extra "title_bar" setting since there's no support? Does it just ignore that setting in that case?

This is just imo, but I think that the colored title bar fits the theme of Materialize pretty well and I think it should be on by default. Although I agree that there should still be a way to turn it off, so a "material_theme_title_bar" option would be good, but maybe have it true by default.

What are your thoughts on it?

[paulhirschi]

Since Windows/Linux does not support this feature, they will simply ignore the "title_bar" class in each .sublime-theme file with no side effects.

I like your idea of having a Theme option for this which defaults to true. I'm not sure how to wire that up, but I'll have a look around at some code - I'm sure I'll be able to figure it out. Thanks for the feedback. I'll be in touch soon.

[paulhirschi]

I discovered a bit in my research for this today:

For each theme, I set "platforms": ["osx"] for the "title_bar" class. Though windows and linux showed no side effects from including the "title_bar" class in each theme, this will help future proof this style from ever creeping into the other platforms and causing unexpected behavior.

I also set the "title_bar" class to only apply when the setting "material_theme_title_bar" is true. The only way I could find to default this setting to true was to add a Preferences.sublime-settings file into the root directory and set this setting there. With this in place, setting "material_theme_title_bar": false in User preferences still has the expected behavior.

If you are unhappy with the addition of the extra Preferences.sublime-settings file in the root directory, I can remove it. Each user would then just have to turn it on individually in their User preferences. If you have any ideas of a better way to go about enabling this by default, let me know.

I'm open to any additional feedback or insight you have.

If you are happy so far, I can go ahead and add "material_theme_title_bar" into the Theme Options section of the README (being sure to mention it is OSX specific and enabled by default) and add documentation anywhere else its needed.

[paulhirschi]

@saadq Another thought - I currently have the foreground (text) of the title bar set to either Black for light themes or White for dark themes. If you'd rather - and I think this would look a bit sharper - I could set the text color of the title bar to match, say, the sidebar label text color. That would be more in keeping with the overall theme as well. I'll go ahead and do that and add some screenshots to get your feedback.

[paulhirschi]

@saadq updated title_bar fg (text) colors to match theme colors. Takes from class: sidebar_label parent attribute: expandable. Line ~586 in most themes.

Let me know how you feel about this and what other work I can do to push this forward. Thanks.

Theme: Toy Chest

Theme: Primer Light

[saadq]

Thanks very much for your work! I'm going to review this in-depth tomorrow, but it's looking really good :)

[saadq]

Alright, so I just went over this. Everything looks really good – the one thing I'd like to see changed is having the title bar's color be different than the background color. Right now, I believe you have the title bar's background color to be set to the background color of the contrast mode. Instead, it would probably make sense to keep the background color of the title bar to be consistent with the theme. If the material_theme_contrast_mode option is enabled, then you can make the title bar's background color the contrast version.

For example, with Material Spacegray you can have this at the top:

{
  "class": "title_bar",
  "settings": ["material_theme_title_bar"],
  "platforms": ["osx"],
  "fg": [175, 189, 196],
  "bg": [43, 48, 59]
}

but then in the contrast mode options (in line 3355 in Material Spacegray.sublime-theme for example), it should have this:

{
  "class": "title_bar",
  "settings": ["material_theme_contrast_mode"],
  "platforms": ["osx"],
  "fg": [175, 189, 196],
  "bg": [41, 45, 55]
}

Does that make sense? Let me know if I can clarify.

[paulhirschi]

@saadq That makes good sense. I'll work on those changes.

[saadq]

Ah woops I meant "settings": ["material_theme_contrast_mode", "material_theme_title_bar"] for the contrast mode thing, but yeah haha sounds good

[skbolton]

Super excited for this change. The colored titlebar is one of my favorite features of themes

[paulhirschi]

Sorry I haven't completed this yet - I've been traveling for work more than usual lately. I'll complete these changes by this weekend.

[saadq]

Sure thing, take your time and let me know if I can help at all or if you have any questions. Great work so far, looking forward to see this in Materialize :)

[rudedogg]

Looking forward to this too! Any news on getting it merged?

[saadq]

Hey @rudedogg I'm ready to merge whenever it is finished. Currently, this PR makes the title bar the color of the contrast mode. I requested changes to make the title bar color the same color as the theme, but use the contrast mode color when material_theme_contrast_mode is enabled. I mentioned above how this would be done.

I imagine @paulhirschi has been busy, but I can always accept a PR from anyone else who would want to implement this if he lets me know that he doesn't have time for it.

[paulhirschi]

@saadq @rudedogg Hey - this fell off my radar a bit. Had some business travel right after I began and spent quite a bit of time away from home. There isn't much work to make it ready - I'll make time to get this finished up and ready this coming week. Thanks for your patience!

[rudedogg]

@saadq @paulhirschi Awesome! Happy to hear you're gonna pick it back up.

If something comes up let me know, I'll give it a go!

[connorlanewhite]

Hey @paulhirschi, speaking for all those that found this via google - you think you'd have time to implement the recommended changes anytime soon?