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

Draw transparent rounded rect ontop of icon when using windowHints, addressing issue Issue #183 #193

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

josh-
Copy link
Contributor

@josh- josh- commented Dec 28, 2012

As noted by @slavick in #183, the window hints are currently quite difficult to read when drawn on lighter icons.

This change draws the transparent rounded rect infront of the icon, instead of behind it.

Some screenshots:
MacVim TextEdit Xcode

I'd like to get @trishume's thoughts on this too - since he was originally the one who implimented icons in window hints.

Also, some constants might need their name changed to reflect the updated position of the rounded rect.

@jigish
Copy link
Owner

jigish commented Dec 28, 2012

@josh- I like this a lot, but in order to maintain configurability, do you think you could divorce this from the original background rounded rect? I'd like to keep the option for both.

@josh-
Copy link
Contributor Author

josh- commented Dec 28, 2012

@jigish Yup, that sounds like an even better idea. Should I add a new config directive?

Also, assuming someone configured Slate to draw the rounded rect behind the icon (as it is in the current shipping version) should a shadow/background be applied to the text? (Thinking of ways to improve readability).

@trishume
Copy link
Contributor

I would prefer that drawing in front simply became the default.
It does not affect people who don't draw icons and drawing behind the icons looks awful.

I thought the last pull request that enabled the rectangles would draw in front of the icons but when it didn't I immediately set their alpha to transparent so that I wouldn't rip my eyes out.

Thank you for fixing this! I think I made a 5 minute attempt and just gave up when I hit a roadblock.
I can't see why anyone would prefer the old behavior. I think this should be merged as-is.

@trishume
Copy link
Contributor

Hmmm just looked at the code and the only thing I think I might add a config option for is the rectangles being slightly smaller than the icon.

Personally I think it looks awesome but I can see that some people may prefer it full size. Also I think it may change the sizing for people not using the icon as well.

@trishume
Copy link
Contributor

trishume commented Aug 1, 2013

This should be merged at some time soon because it is amazing, even if it is not fully configurable.

@p0deje
Copy link

p0deje commented Dec 9, 2013

👍

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.

4 participants