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

ui(compact-bar): mode indicator not centered #3248

Closed
cristiand391 opened this issue Apr 6, 2024 · 5 comments · Fixed by #3260
Closed

ui(compact-bar): mode indicator not centered #3248

cristiand391 opened this issue Apr 6, 2024 · 5 comments · Fixed by #3260
Labels
suspected bug

Comments

@cristiand391
Copy link
Contributor

2. Issues with the Zellij UI / behavior / crash

Issue description

I noticed the mode indicator isn't correctly centered when the mode name length > 6, you can see in this video how when switching to renametab/renamepane the text isn't padded (IIRC it's not noticeable on the left side because the Zellij (session-name) section includes a whitespace).
Screencast from 04-06-2024 05:32:06 PM.webm

This is how I improved the padding in my fork of compact-bar, you can see the mode name is always centered (even short ones like pane):
Screencast from 04-06-2024 05:44:54 PM.webm

cristiand391/zj-status-bar@2de3d60

If the current padding behavior in zellij's compact-bar isn't the intended and the maintainer agrees, I'm happy to contribute the fix upstream.

Minimal reproduction

save this as layout

layout {
  pane size=1 borderless=true {
    plugin location="zellij:compact-bar"
  }
  pane
}

switch to renametab or any mode name > 6:

zellij action switch-mode renametab

image

Other relevant information

zellij --version
zellij 0.39.2

stty size
35 159
@Zykino
Copy link
Contributor

Zykino commented Apr 6, 2024 •
edited
Loading

Love that fix, looks elegant to me. Not sure what maintainers would think of it.

Edit: Just thinking about it, another/easier fix would be: let mode_part_padded = format!(" {} ", mode_part); isn’t it?

@cristiand391
Copy link
Contributor Author

Just thinking about it, another/easier fix would be: let mode_part_padded = format!(" {} ", mode_part); isn’t it?

ha, yes that should work too, good catch. I probably went with the aligment solution because I read that was a thing in the docs 😄
https://doc.rust-lang.org/std/fmt/index.html#fillalignment

@Zykino
Copy link
Contributor

Zykino commented Apr 6, 2024

Yup, and the starting code you had was like this, so I think I would have done the same at first too ^^.

@jaeheonji
Copy link
Member

jaeheonji commented Apr 10, 2024 •
edited
Loading

@cristiand391

That's great!, I love it too. It seems like your code can cover a variety of cases considering the two empty paddings. If possible, can you make a PR?

@cristiand391 cristiand391 mentioned this issue Apr 12, 2024
Merged
@cristiand391
Copy link
Contributor Author

@jaeheonji
Here it is: #3260
with @Zykino's suggestion to just use whitespaces instead of dynamic alignment :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
suspected bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(compact-bar): properly pad mode indicator cristiand391/zellij
3 participants
@Zykino @cristiand391 @jaeheonji