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

Bracket positioning, padding between brackets. #351

Closed
crissNb opened this issue Apr 4, 2023 · 10 comments
Closed

Bracket positioning, padding between brackets. #351

crissNb opened this issue Apr 4, 2023 · 10 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@crissNb
Copy link

crissNb commented Apr 4, 2023

It would be great if there is a way to add padding between two brackets. I've tried background.padding_left and background.padding_right, but this had no effect on padding. Maybe I'm doing something wrong, or it's already implemented in another way.
According to the docs, it seems like "Brackets currently only support all background features.", so if it isn't implemented already, I'd love to see this feature coming.

It also seems like if I have multiple brackets containing items that are anchored to the right (so the bracket is "positioned" right), an item that is anchored to the middle gets shifted towards left slightly. I'm saying "slightly" here, because the amount that it gets shifted isn't same as the width of the bracket (it's much less), which seems quite weird.

Part of me is also wondering if I've correctly understood the concept of brackets. I've thought of brackets as another item that simply groups multiple items together with a common background.

@FelixKratz
Copy link
Owner

It would be great if there is a way to add padding between two brackets. I've tried background.padding_left and background.padding_right, but this had no effect on padding. Maybe I'm doing something wrong, or it's already implemented in another way. According to the docs, it seems like "Brackets currently only support all background features.", so if it isn't implemented already, I'd love to see this feature coming.

Thats a good idea, should be doable. I will have a look some time soon.

It also seems like if I have multiple brackets containing items that are anchored to the right (so the bracket is "positioned" right), an item that is anchored to the middle gets shifted towards left slightly. I'm saying "slightly" here, because the amount that it gets shifted isn't same as the width of the bracket (it's much less), which seems quite weird.

This sounds like a bug, probably something in the centering calculations does not quite check out. Can you provide a minimal example to reproduce?

Part of me is also wondering if I've correctly understood the concept of brackets. I've thought of brackets as another item that simply groups multiple items together with a common background.

This is exactly what brackets should be doing: Brackets provide a common background bounded by the outermost visible members of the bracket.

@FelixKratz FelixKratz added bug Something isn't working enhancement New feature or request labels Apr 5, 2023
@crissNb
Copy link
Author

crissNb commented Apr 5, 2023

This sounds like a bug, probably something in the centering calculations does not quite check out. Can you provide a minimal example to reproduce?

I've actually done further testing and (at least) on my end, the behavior is super strange... The centered item gets shifted to left, if:

  1. there's a bracket on the right side
  2. I know this is absurd, but an item that is contained in the bracket starts with the word "calendar" (?)
  3. default icon.padding needs to be greater than 0.

I've been trying to figure out why the second condition needs to be met. Do you have any ideas? Let me know if this is even reproducible on your end. To be honest, I feel like this isn't a bug directly related to SketchyBar, rather a misunderstanding I have.
Something I've also noticed, is that the shifting only happens a split second after the sketchybar has been loaded (see attached screen recording).

weird.shifting.mov

Here's the minimal example config I used (bare sketchybarrc):

sketchybar --default icon.padding_right=20       \
                     icon.padding_left=20
centered_item=(
    width=300
    background.color=0xfff38ba8
    label="CENTER"
    label.align=center
)

sketchybar --add item centered center            \
           --set centered "${centered_item[@]}"

test_item=(
    width=140
)

sketchybar --add item     calendar123123 right    \
           --set calendar123123 "${test_item[@]}"

sketchybar --add bracket bracket.1 calendar123123

@FelixKratz
Copy link
Owner

I think I know why it is happening, it should be a one line fix. It is happening because of the c as the first character in calendar. Any other item beginning with c should show the same problem.

@FelixKratz
Copy link
Owner

The centering bug should now be fixed.

@FelixKratz
Copy link
Owner

FelixKratz commented Apr 5, 2023

The bracket paddings should be respected now: 44cb73e
Can you test it and report any new issues? I guess this will change the visuals of some configs sightly, since the padding_left and padding_right are defaulted in most setups, changing the padding on those brackets essentially from 0 to whatever is in the defaults (but I think the way it is now is the proper expected behavior).

@crissNb
Copy link
Author

crissNb commented Apr 5, 2023

There seem to be another issue. Whenever the width (or "drawing" property also seemed to be causing the issue, so my guess is that it happens with every property) of an item changes (this item doesn't necessarily need to be centered, also happens if the item is anchored to left or right), items that are contained in a bracket seem to be randomly moving. This problem is most noticeable, if I have stacked items (one item is above another and both items have width of zero), as items move left/right randomly. Cool thing is that there is a jitter motion if the width is being modified via an animation. This didn't happen in the "previous" version (latest stable).

In order to reproduce the issue, the following conditions need to be met according to my testing:

  1. associated_display of a moving item need to be set. I've tried "active" and this caused the issue.
  2. background.padding_left/background.padding_right need to be set.

Here's a screen recording of the issue. As you can see, the items that are located top right corner moves randomly as I modify the width of the centered item.

jitter.mov

Here's the minimal example config I used (bare sketchybarrc):

sketchybar --default background.padding_right=5 \
                     background.padding_left=5

centered_item=(
    width=300
    background.color=0xfff38ba8
    label="CENTER"
    label.align=center
    associated_display=active
)


sketchybar --add item centered center           \
           --set centered "${centered_item[@]}"


item_down=(
    width=0
    y_offset=-5
    label="down"
)

item_up=(
    width=0
    y_offset=5
    label="up"
)

sketchybar --add item     test.1 right         \
           --set test.1 "${item_down[@]}"
sketchybar --add item     test.2 right         \
           --set test.2 "${item_up[@]}"

sketchybar --add bracket bracket.1 test.1 test.2

FelixKratz added a commit that referenced this issue Apr 6, 2023
@FelixKratz
Copy link
Owner

FelixKratz commented Apr 6, 2023

Should be fixed, but it is a bit more involved than I originally thought, even the current logic has some problems with brackets that span between items positioned left and right in the bar.

@FelixKratz
Copy link
Owner

I had to revert the changes made for this feature, because the implementation was not robust enough. The problem with this feature is that to make it fully robust significant computational overhead has to be introduced.

The problem is: Brackets span between the leftmost and the rightmost item in an unordered member list. There is no way to know for sure which item is e.g. leftmost before calculating all the positions first without the bracket paddings to get the info which item should have the additional paddings, and then do a second pass through the position calculation to determine the correct positions for all the items with the bracket padding included.

@FelixKratz
Copy link
Owner

FelixKratz commented Apr 22, 2023

The bug is fixed in the latest release, I will close this for now and think about the bracket padding problem a bit longer.

@Pe8er
Copy link

Pe8er commented Jun 1, 2023

@crissNb a stopgap solution for padding between brackets is to add a "spacer" item:

(bracket 1 items)

--add item spacer right                \
--set spacer background.drawing=off    \
             width=4                   \

(bracket 2 items)

Works well for me, hope you find it useful!

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

No branches or pull requests

3 participants