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.

Sign up for GitHub

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

Jump to bottom

min_width padding should be added after the label, not including it #219

Closed
roobre opened this issue Feb 5, 2017 · 4 comments
Closed

min_width padding should be added after the label, not including it #219

roobre opened this issue Feb 5, 2017 · 4 comments

Comments

@roobre
Copy link
Contributor

roobre commented Feb 5, 2017

I've found that the min_width directive includes the label size when calculating the actual minimum width of a block. This is certainly counter-intuitive, and cause problems with icon fonts (like ttf-font-awesome), as these characters usually don't have the same width as alphanumeric characters.

This behaviour also cause the label to move right and left, instead of staying still. See attached pictures of the cpu_usage script for an example:

tmpopgl5r-save
tmpkwanzg-save

I think the size of the block label shouldn't be taken into account for the minimum width directive, as it causes strange behaviour and defeats the purpose of this parameter.
@roobre roobre changed the title min_width padding should be added before the label, not including it min_width padding should be added after the label, not including it Feb 5, 2017
@vivien
Copy link
Owner

vivien commented Sep 10, 2018 •
edited
Loading

The label property is specific to i3blocks (not i3bar) and was added for convenience to easily add a prefix text to full_text. But this causes inconsistencies as the one you described.

This property will be deprecated, especially since arbitrary properties are now supported and can be handled directly within the script. So one will want to either include the label from the script if $label (or any preferred name) is non-empty, or, adding a static block without separator on its left.

@vivien vivien closed this as completed Sep 10, 2018
vivien added a commit that referenced this issue Sep 10, 2018
@vivien
block: remove support for label
231829b 
The "label" property was specific to i3blocks and implemented as an easy
way to prefix the "full_text" property. However it brings some
inconsistencies to the user, such as the calculation of the "min_width".

For this reason, The support for the "label" key is removed.
Below are examples how to properly add a label for a "time" block.

Either you can keep the "label" key and use its value in the command:

    [time]
    label=Time:
    command=echo -n "$label"; date +%T
    interval=30

Or more cleanly with a static block on its left:

    [time-label]
    separator_block_width=0
    separator=false
    full_text=Time:

    [time]
    command=date +%T
    interval=30

Refs #219
vivien added a commit that referenced this issue Sep 10, 2018
@vivien
block: remove support for label
61646f2 
The "label" property was specific to i3blocks and implemented as an easy
way to prefix the "full_text" property. However it brings some
inconsistencies to the user, such as the calculation of the "min_width".

For this reason, The support for the "label" key is removed.
Below are examples how to properly add a label for a "time" block.

Either you can keep the "label" key and use its value in the command:

    [time]
    label=Time:
    command=echo -n "$label"; date +%T
    interval=30

Or more cleanly with a static block on its left:

    [time-label]
    separator_block_width=0
    separator=false
    full_text=Time:

    [time]
    command=date +%T
    interval=30

Refs #219
@helge000
Copy link

helge000 commented Oct 8, 2018 •
edited
Loading

@vivien, I updated i3blocks just recently and had to do a git bisect to find 61646f2. Mind you, there is a difference between deprecate and remove.

Please also add migration guides or/and deprecation warnings in the future, not just commit messages.
Thanks,

@vivien
Copy link
Owner

vivien commented Oct 8, 2018

Hi @helge000, you are correct. In the end I considered removing it not a breakage, but I understand that it can bring a frustrating experience. I will restore support for label for the next release.

vivien added a commit that referenced this issue Oct 8, 2018
@vivien
block: restore deprecated label
37f2380 
Now that user variables are supported, it is preferable that a blocklet
handles its label on its own, even more since this brings inconsistency
with the full_text length calculation.

But at the same time, removing its support right now can bring some
frustration to the user as well as a bad experience. Now that we
store unescaped JSON values again, bring back the support for the
label property and deprecate it for the duration of the next release.

Refs #219
@helge000
Copy link

helge000 commented Oct 9, 2018

@vivien , thanks!

@zsugabubus zsugabubus mentioned this issue Jun 16, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vivien @roobre @helge000