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

Indicator not shown when stack width < minimum width of stacked window. [Original: Iterm icon is not visible] #32

Closed
Coobaha opened this issue Aug 29, 2020 · 2 comments · Fixed by #37
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@Coobaha
Copy link

Coobaha commented Aug 29, 2020

Hi @AdamWagner

First of all thanks for writing this scripts :) nice addition to yabai 💯

Grouping by stackId is buggy. Assumption to make stackId based on x+y+w+h is incorrect, as some windows can have different dimensions (for instance terminal or iterm and any other app that have weird behaviour when very narrow).

stackId = table.concat({x, y, w, h}, '|'),

Replacing

byStack = u.filter(u.groupBy(windows, 'stackId'), u.greaterThan(1)) -- stacks have >1 window, so ignore 'groups' of 1

with

byStack = u.groupBy(windows, 'stackId')

Solves it, but not sure if this is a correct fix. At least it works for me :) Could make a PR if this is the right way to fix it

Before
image

After:
image

UPD: Nah naive fix makes it more buggy :/

@AdamWagner
Copy link
Owner

@Coobaha Thanks for the issue.

Grouping by stackId is buggy. Assumption to make stackId based on x+y+w+h is incorrect, as some windows can have different dimensions (for instance terminal or iterm and any other app that have weird behaviour when very narrow).

You're absolutely right that grouping by stackId (which is really just a string representation of the window frame) is not always reliable, and it's good to call it out here so that others know.

My personal, interim solution for this is to not make stacks that are very narrow. The minimum window size in macOS is generally an annoyance, and that continues to e true here.


UPD: Nah naive fix makes it more buggy :/

Yep :-) We'll want to continue filtering out stacks that only contain 1 window.


I do have an idea for how we can improve this, though:

Fuzz the stackId. It would be relatively simple to just round window dimensions to the nearest number divisible by 5 (or 10), right before stringifying the window frame into the stackId. This would probably fix the issue, and be unlikely to produce false positives. It would still fail with really skinny stacks that are more than fuzzFactor less than one of the windows' minimum size, but I'm inclined to just call this out as an unsupported case.

What do you think about ↑ ?

@AdamWagner AdamWagner changed the title Iterm icon is not visible Indicator not shown when stack width < minimum width of stacked window. [Original: Iterm icon is not visible] Sep 1, 2020
@AdamWagner AdamWagner added bug Something isn't working documentation Improvements or additions to documentation labels Sep 1, 2020
@Coobaha
Copy link
Author

Coobaha commented Sep 1, 2020

@AdamWagner yea this probably should fix it, great idea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants