search

zellij-org / zellij

A terminal workspace with batteries included
https://zellij.dev
MIT License
20.47k stars 637 forks source link

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

Closed cristiand391 closed 4 months ago

cristiand391 commented 5 months ago

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

https://github.com/cristiand391/zj-status-bar/commit/2de3d60446dfe8fe9d9037122d77daf927be8d7d

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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

jaeheonji commented 5 months ago

@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 commented 4 months ago

@jaeheonji Here it is: https://github.com/zellij-org/zellij/pull/3260 with @Zykino's suggestion to just use whitespaces instead of dynamic alignment :)