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

Add support for dynamic colors #17

Merged
merged 4 commits into from
Aug 13, 2020

Conversation

alin23
Copy link
Contributor

@alin23 alin23 commented Aug 7, 2020

Closes #16
I also ran ImageOptim on all the assets to get the to a lower size so README loads faster.

Preview on light backgrounds

image

Preview on mixed backgrounds

stackline-color-indicators

@AdamWagner
Copy link
Owner

This 👏 looks 👏 so 👏 SICK!

I'm mildly jealous :-)

Unfortunately, I wasn't able to get the dynamic bg color to work for me. It seems like it defaults to a dark bg color. And note in the attached screen recording that against a pure black background, the indicators remained at their default dark color and are almost invisible against the background.

stackline-dynamic-colors-on-dark-background

I cloned your repo, completely unlinked my version, added the yabai signals back to yabairc. I also switched my wallpaper several times, hoping to trigger the wallpaper watcher, but the indicators did not appear to respond.

Did I miss a step? Or, do you have an idea of what could be causing the indicators to (seemingly) ignore my wallpaper?

I was able to get make the active indicator red by issuing the echo ":indicator_colors:1" | hs -m stackline-config command. This is cool!

And good call running image optim on the pngs - thanks!

@alin23
Copy link
Contributor Author

alin23 commented Aug 12, 2020

Yep, there were a few bugs in calculating the coordinates and in initializing the watcher. Everything should work now, make sure Hammerspoon has Recording permissions before trying this again:
image

It should ask for the permissions on the first hs.screen:snapshot call but just to be sure, check that again 😅

@AdamWagner
Copy link
Owner

@alin23 This is incredible stuff! When I opened #16 I had no idea that such a good solution was even possible. I'm so excited for this to be a feature!

(Hammerspoon already had screen recording permission, so I haven't tested that it will request it as expected).

I'm going to merge this.

I am a bit discouraged by the general janky-ness of the underlying functionality in this version, which makes it harder to test just this awesome new functionality, but that's no fault of yours! I'm just anxious to get #18 into a mergeable state.

@AdamWagner AdamWagner merged commit e116a49 into AdamWagner:master Aug 13, 2020
@AdamWagner
Copy link
Owner

@alin23 So… I messed up.

Sometime last weekend, I think I force-pushed in a panic and overwrote the changes in this PR. I'm really sorry.

I'd be happy to sift through the changes here to add the functionality back to master, (which now includes the "Query with Hammerspoon" changes), but these is your awesome feature, and it shouldn't be associated with my account…

Maybe if you just open another PR, I could work out the merge conflicts and the functionality commits would still be attributed to you?

@alin23
Copy link
Contributor Author

alin23 commented Aug 22, 2020

@AdamWagner hey no worries! I still have the commit on my branch, I'll open another PR if I get some time this weekend.

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.

Indicator color does not auto-adjust based on avg. luminosity of wallpaper to ensure legibility
2 participants