-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Feature - Toggle: Only apply transparency to default bg #277
Feature - Toggle: Only apply transparency to default bg #277
Conversation
Really cool 😄 |
<outlet property="nextKeyView" destination="fwj-4Y-nja" id="u8P-d1-x0a"/> | ||
</connections> | ||
</button> | ||
<button toolTip="Enable this to prevent background colors for text from being affected by the transparency setting. Useful for having a transparent terminal background but opaque Powerline segments(background) to match opaque Powerline separators(foreground)." id="fwj-4Y-nja"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This copy is a little stilted. Try:
Enable this to make non-default background colors always opaque. This is useful when using Powerline in a translucent window.
As per request: gnachman#277 (comment)
As per request: gnachman#277 (comment)
to `setTransparencyAffectsOnlyDefaultBackgroundColor`. As per request: gnachman#277 (comment)
As per request: gnachman#277 (comment)
@gnachman Updated with your change requests. Did you want to update the key |
<button toolTip="Enable this to make non-default background colors always opaque. This is useful when using Powerline in a translucent window." id="fwj-4Y-nja"> | ||
<rect key="frame" x="15" y="9" width="389" height="18"/> | ||
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMinY="YES"/> | ||
<buttonCell key="cell" type="check" title="Only apply transparency to the main background color." bezelStyle="regularSquare" imagePosition="left" alignment="left" inset="2" id="sBm-XA-tIi"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change "main" to "default" here
Please rebase so I can merge this after the next batch of changes. |
A conditional check for non default BG colors will prevent alpha being applied to them. `_defaultBGAlphaOnly` will reference a user setting(checkbox) to make this feature opt-in.
Height is adjusted for the `77f-eM-Dma`/`Prefs - Profiles - Window` to make room for new checkbox element. Doing so adjusted all the `y` position values.
I added the checkbox with copy/paste of the previous element within the XCode UI editor and updated the properties. I could not find how to add the `outlet` hunk via that interface so directly added it by editing the XML. The id I used for the `outlet` node doesn't have any conflicts and is presumably safe.
Points the `_preventTab` node to `_defaultBGAlphaOnly` node. Also added a `nextKeyView` connection from `syncTitle` to `_preventTab` as it was missing one.
This finishes the feature off by connecting the UI checkbox setting to the first commit that introduced the `_defaultBGAlphaOnly` variable. I've tried to keep the location of additions consistent, either being inserted after `blend` or `_preventDefault` logic.
As per request: gnachman#277 (comment)
As per request: gnachman#277 (comment)
to `setTransparencyAffectsOnlyDefaultBackgroundColor`. As per request: gnachman#277 (comment)
As per request: gnachman#277 (comment)
Use American English spelling.
As per request: gnachman#277 (comment)
72edd76
to
22d5b03
Compare
to `KEY_TRANSPARENCY_AFFECTS_ONLY_DEFAULT_BACKGROUND_COLOR`. As per request: gnachman#277 (comment)
to `KEY_TRANSPARENCY_AFFECTS_ONLY_DEFAULT_BACKGROUND_COLOR`. As per request: gnachman#277 (comment)
22d5b03
to
38c38f7
Compare
This was my first rebase with a pull request. Hopefully I did it right with the force push. I missed a file with the last commit, I assume it's ok to ammend and force push in this case.
I'm not sure why Travis/XCode is warning about this, is it due to length? |
Merged, thanks! |
Awesome feature, was waiting for it! Thanks a lot :) |
I'm building in xcode but I can't seem to find the option under the Profiles/Window tab. Help! Update: I'm fairly certain that the commit has NOT been merged in. In addition to this, when I do merge it in, it does not work. Well, it does not work if I merge it So I'm just running polarathene's branch. Hopefully someone can comment on what the status of things are and resolve this. |
@unphased My commit is in the master branch history, it's a single merge commit, you can see it here. Here is a compare view between that commit and the latest commit on master branch currently. I've had it select/view a line that has introduced a small refactor to my work since it was committed. There doesn't appear to be anything else deleted/modified since then besides moving a Looking at the linked files history, right now there is just a single commit after this PR's merge commit. Build from master branch at the merge commit and see if that works for you, if it does, then build the commit that refactored that code. If it no longer works, then a bug was introduced. |
@polarathene I see it now. All that was needed to confuse me was that the new setting shows up in a new spot near the top under the Transparency slider (which is not in the Misc section as you show in your original branch). Everything's working great. |
@unphased I've yet to update the official iTerm2, I wasn't aware it had also been relocated, sorry about that. |
I moved some things around and made other improvements to that settings pane, sorry for not mentioning it in here. |
@polarathene @gnachman Thanks for this, it works great. But I guess we need something like this for dimming panes too, look at this: |
Would be garish for the red to not get dimmed, don't you think? |
@gnachman I don't think @sassanh meant that. The dimming is inconsistent between the background and foreground colours, just like with transparency prior to the patch I made in this PR. I have an animated example at the top that shows the segments/separators inconsistency when applying transparency. While that favours visibility by not making them transparent, @sassanh is probably wanting the colours to remain consistent for the segment/separators, so both should dim? I no longer have access to a Mac or use iterm2 at present, so I'm unable to look into this. |
@polarathene yeah, that's what I meant. |
What's wrong with powerline fonts? I mean why they don't behave like normal fonts? |
Oh, I see, the issue is that the left arrow isn't dimmed the same as the red to the right of it. |
Hm, dims OK for me, but I'm sure our settings are different. @sassanh please file an issue at https://iterm2.com/bugs. |
Sure! |
@polarathene I can't find the toggle. Is it removed? |
@nneekro I no longer have access to a system with macOS and haven' t used iTerm2 for several years as a result, so no clue 😕 |
@gnachman thanks |
@EduardoOliveira Are you referring to the triangles at the bottom? They should be opaque because they're foregorund, not background. You might want to turn on "Keep background colors opaque" under "transparency" |
That's it thanks |
This fixes a recent issue I raised here and it's related June 2013 issue. The problem is also brought up in other repo's and communities. The main advantage being Powerline themes that don't break from having transparent backgrounds.
I've added a UI element to opt-in to the feature, it should not cause any problems for those not interested(Although it's great for other cases such as messages like session restore text being more readable on a solid grey instead of transparent ;) ). See each commit message for more information :)
Here's a comparison with and without: