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 configurable padding and focus text for some widgets #215

Merged
merged 9 commits into from
Jun 28, 2021

Conversation

schiltz3
Copy link
Contributor

@schiltz3 schiltz3 commented Jun 27, 2021

Add attributes to the standard widgets allowing their text and padding to be modified in the config.

NOTE: The added attributes have default values corresponding to their original hard-coded values, so this is backward compatible.


1. Edit or remove padding characters around edges of widget elements:

Before:    image

After:     image

2. Change active monitor character and active layout padding

Before:    image

After:     image

@N1x0
Copy link
Contributor

N1x0 commented Jun 27, 2021

I like the ability to set padding, I'd just move it to IBarWidgetPart and then implement in the individual widgets as necessary. Also IMO defaults should not have any padding, I know this is up to personal preference, just stating mine.

If you could make the necessary changes I'll approve them, thanks for the PR!

@dalyIsaac dalyIsaac changed the title Additional widget customization Add configurable padding and focus text for some widgets Jun 28, 2021
@schiltz3
Copy link
Contributor Author

I believe I made the changes you suggested.

The default padding has been removed. I can add it back in if we think it is outside the scope of this PR.

I did not change the Focus/Unfocus text, but I think it could be changed to the focus text being the text attribute, and hard coding unfocused to be an empty string.

LMK what you think

Copy link
Contributor

@N1x0 N1x0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me, I also agree that FocusedMonitorWidget should return something meaningful instead of just '****' or "", but I think that's better kept in a seperate PR. I will write something in the docs to reflect the change. Please have a look there as well as the author ;)

Copy link
Member

@dalyIsaac dalyIsaac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @N1x0 - otherwise LGTM!

@dalyIsaac dalyIsaac merged commit 541d1cc into workspacer:master Jun 28, 2021
@schiltz3
Copy link
Contributor Author

Changes look good to me, I also agree that FocusedMonitorWidget should return something meaningful instead of just '****' or "".

I may update the docs later with examples of how to use Extended Unicode with workspacer. With fonts like FontAwsome

@rikai-suru
Copy link

I love this PR because since day 1 I've always customized the padding (and needed to recompile workspacer everytime it updates).

However, a padding set to an empty string ("") instead of with a space (" ") leads to this behavior:

padding.issues.mp4

because of an inconsistency in line 88 of WorkspaceWidget.cs:
return visible ? LeftPadding + workspace.Name + RightPadding : $" {workspace.Name} ";

If both padding variables are "", then once you focus on a workspace its length shrinks on the bar.

I would've liked to submit a fix myself, but I'm still not comfortable with Git and programming in general.

@dalyIsaac dalyIsaac added this to the 0.9.11 milestone Jun 29, 2021
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