regolith-linux / regolith-look-extra

Additional Looks for the Regolith Desktop
5 stars 12 forks source link

Improve contrast of active vs inactive tabbed mode windows in nord theme #12

Closed mfisher87 closed 1 year ago

mfisher87 commented 1 year ago

When using i3's "tabbed" layout, I can't differentiate between the active tab and inactive tabs in this theme (this issue may be specific to my eyeballs or my display settings).

I'm not sure if this is the appropriate place to make that change (e.g. is there another setting that I can change to improve contrast by making the inactive tab background use base03 instead of base02 and the active tab background use base00 instead of base01?), unsure if base03 and base02 are used together anywhere (this PR makes them the same color), and I'm also unsure if it's OK that the nord theme color #3b4252 is no longer represented anywhere. So far I haven't noticed any visual issues.

Old:

image

New:

image

mfisher87 commented 1 year ago

That's a much better approach than my naive attempt :) Thanks for pointing me in the right direction!

mfisher87 commented 1 year ago

Would you mind walking me through how you typically test changes to a look? I'd like to document in README a more optimized workflow than the one I'm using.

My naive (as always) workflow idea was to create a symbolic link from a test directory to the repo look (e.g. ln -s /usr/share/regolith-look/nord-testing ~/path/to/regolith-look-extra/usr/share/regolith-look/nord) so I can (A) make tracked changes in the repo and see changes visually reflected when I refresh the look and (B) retain the ability to switch to the original version of the look installed by apt for comparison. However, such a symlink directory was not picked up by regolith-look-selector (the files are all mode o+r so I don't think it's an ownership issue).

As a fallback, I did a fully copy of an existing theme in /usr/share/regolith-look (and now it is selectable in regolith-look-selector), but replaced the files I'm changing with symlinks to files in the repo. This works, but requires changing the #include directives in root to match the new testing path of the i3wm file (which would also be a negative of the symlinked-directory approach). At this point, I can load my look (or the original apt-installed version) with the selector, make changes, and regolith-look refresh to see them reflected.

mfisher87 commented 1 year ago

Thank you for pointing me in the right direction for the values to edit. I was able to implement a much better (to my eye) solution than what I originally suggested. The inactive/unfocused tabs now display like inactive workspaces. I originally thought maybe this wouldn't be enough contrast, but it's exactly the same as the i3bar text/background colors (e.g. the date and time), and those have looked fine to me for daily work for long enough.

EDIT: Forgot to post screenshot

image

mfisher87 commented 1 year ago

I realized that inactive tab borders were not visible with this change, and no darker colors are available in the nord palette so I selected a lighter color for the borders.

image

kgilmer commented 1 year ago

Are you still working on this PR @mfisher87 or ready for review?

Good call out about regolith-look-selector and symlinks. If you're interested in fixing that, here is what needs to be changed: https://github.com/regolith-linux/regolith-session/blob/main/usr/bin/regolith-look#L94 And I think we just need to add -L to the find command.

mfisher87 commented 1 year ago

Ready for review! I'll have to peek at that link a bit later, but definitely interested in giving it a shot. If I'm able to make that fix, do you think the symlinked directory workflow is a good one to recommend in the README?

On Mon, Jan 2, 2023, 8:53 PM Ken Gilmer @.***> wrote:

Are you still working on this PR @mfisher87 https://github.com/mfisher87 or ready for review?

Good call out about regolith-look-selector and symlinks. If you're interested in fixing that, here is what needs to be changed: https://github.com/regolith-linux/regolith-session/blob/main/usr/bin/regolith-look#L94 And I think we just need to add -L to the find command https://joshtronic.com/2021/06/13/using-find-with-symlinks/.

— Reply to this email directly, view it on GitHub https://github.com/regolith-linux/regolith-look-extra/pull/12#issuecomment-1369368218, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3Q5SC3VSMEQAO5QSLJL3TWQOPCDANCNFSM6AAAAAATH46KBU . You are receiving this because you were mentioned.Message ID: @.***>

kgilmer commented 1 year ago

If I'm able to make that fix, do you think the symlinked directory workflow is a good one to recommend in the README?

Could you provide more info?

mfisher87 commented 1 year ago

I'm not able to log on to github in my browser at the moment, but a few comments up I mentioned adding some contributor workflow documentation to the README (I believe the same one that brought up the issue with regolith-look-selector. Did that comment provide enough context to clarify what I'd like to document?

Thanks again for your time :)

Matt

On Mon, Jan 2, 2023, 9:09 PM Ken Gilmer @.***> wrote:

If I'm able to make that fix, do you think the symlinked directory workflow is a good one to recommend in the README?

Could you provide more info?

— Reply to this email directly, view it on GitHub https://github.com/regolith-linux/regolith-look-extra/pull/12#issuecomment-1369377704, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3Q5SBTOTU2QZ6EDP4RVKTWQOQ57ANCNFSM6AAAAAATH46KBU . You are receiving this because you were mentioned.Message ID: @.***>

kgilmer commented 1 year ago

Whoops, I missed that one sorry.

Would you mind walking me through how you typically test changes to a look?

Well I usually just do what you ended up doing. But, there is another way from Regolith 2.1 (although it needs to be better documented); regolith-look-selector will source looks from your homedir (~/.config/regolith2/looks) so your dev workflow sounds good with that minor tweak and is much better than muddling w/ root-owned files in /usr (IMO).

mfisher87 commented 1 year ago

Wonderful, thanks! That is much better than working in /usr/share! Will submit a doc PR and a fix for the linked bash script in the coming days :)

Matt

On Mon, Jan 2, 2023, 9:22 PM Ken Gilmer @.***> wrote:

Whoops, I missed that one sorry.

Would you mind walking me through how you typically test changes to a look?

Well I usually just do what you ended up doing. But, there is another way from Regolith 2.1 (although it needs to be better documented); regolith-look-selector will source looks from your homedir ( ~/.config/regolith2/looks) so your dev workflow sounds good with that minor tweak and is much better than muddling w/ root-owned files in /usr (IMO).

— Reply to this email directly, view it on GitHub https://github.com/regolith-linux/regolith-look-extra/pull/12#issuecomment-1369383821, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3Q5SCBGJIAXII7TVCKCOTWQOSRLANCNFSM6AAAAAATH46KBU . You are receiving this because you were mentioned.Message ID: @.***>

mfisher87 commented 1 year ago

What do you think about prescribing in the README workflow section the use of relative rather than absolute paths for #include directives? I tested this and it worked as I expected. E.g.: #include "./i3-wm" instead of #include "/usr/share/regolith-look/nord/i3-wm". While I'm updating the doc, can also update the existing looks to use relative paths where appropriate.

This would enable a workflow where a contributor creates a symlink at ~/.config/regolith2/looks/<any_name> pointing to the directory for the look they're working on in their local copy of this repository. That way testing from the home directory would work with the same includes as if the look were deployed to /usr/share/regolith-look.

kgilmer commented 1 year ago

That sounds great @mfisher87 . Assuming no functional regressions that is a better way to go than absolute paths :+1: