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

feat(ui): round frame corners #669

Closed
wants to merge 2 commits into from
Closed

feat(ui): round frame corners #669

wants to merge 2 commits into from

Conversation

TheLostLambda
Copy link
Member

I think rounded corners look nice! This PR adds them by default:
Screenshot from 2021-08-29 21-19-34

For our font-deficient friends, we fall back to square corners with --simplified-ui:
Screenshot from 2021-08-29 21-21-09

Let me know what you think and if things display correctly with your font!

@a-kenji
Copy link
Contributor

a-kenji commented Aug 30, 2021

I agree, this looks very nice, I personally would vote for a separate option and to not make that the default, because of the font issue.

To tie it to simplified_ui is not a bad idea, but it could be an even worse first experience for people that don't just have extended fonts installed.

@imsnif
Copy link
Member

imsnif commented Aug 30, 2021

Fully agree with @a-kenji

@TheLostLambda
Copy link
Member Author

TheLostLambda commented Aug 30, 2021

Okay, then I don't think it's really worth a command-line flag – should I just add an option in the config that defaults to false? A rounded-corners option?

@TheLostLambda
Copy link
Member Author

On second thought @a-kenji and @imsnif , wouldn't anywhere the corners are broken also have broken status and tab bars? We have the fancy status bars by default, is this that much different? Our defaults kinda already require fonts with these nice characters, don't they?

@imsnif
Copy link
Member

imsnif commented Aug 30, 2021

I don't know if every font that has the arrow characters also has the rounded corners. :/ When I made the arrow characters I was sure all the fonts have them. I don't know how to check the validity of this.

@TheLostLambda
Copy link
Member Author

I don't know if every font that has the arrow characters also has the rounded corners. :/ When I made the arrow characters I was sure all the fonts have them. I don't know how to check the validity of this.

That's a valid concern. I'll do some testing of different fonts and Linux distributions if you think that would be helpful? How were you sure that all fonts had arrow characters?

@imsnif
Copy link
Member

imsnif commented Aug 30, 2021

I just didn't think about it to be honest. If you want to take responsibility for this, then go for it. Just be aware that:

  1. If some fonts don't have this, we'll likely lose users (this is a very bad first experience)
  2. I personally will not know how to check if your tests were conclusive enough.

@a-kenji
Copy link
Contributor

a-kenji commented Aug 30, 2021

@TheLostLambda,
No I think you are right, I personally think this is a little different here though.
While it is a default we ship currently, it is in the default plugins and not the default application,
while that is currently a very arbitrary difference maybe that will not be in the future.
I am not sure if that warrants a different look at things.

But starting zellij without any plugins by default currently does not require certain fonts to be installed.

@TheLostLambda
Copy link
Member Author

TheLostLambda commented Sep 6, 2021

For this, are people thinking that we should not make it the default and have a separate option for it? I'd personally be comfortable making it the default and would probably prefer that (I never ran into missing fonts accept on the TTY after testing half a dozen distros – most of which did not have the arrow fonts), but my resolve has softened enough to keep the square corners the default if others are particularly set on that.

@imsnif
Copy link
Member

imsnif commented Sep 6, 2021

For me: as I mentioned offline, I'd feel confident enough to make this the default if I understood more. Why doesn't the TTY font have it, for example? Where's the difference?

Even if it were just the TTY though - the benefit seems so small to me in making this the default (I personally would not be able to tell the difference at a glance, but maybe it's my lack of aesthetic sense :) ), versus the risk of losing even one user to it not working out of the box as they would expect it to, that my vote is no.

That's really just my opinion though.

@a-kenji
Copy link
Contributor

a-kenji commented Nov 1, 2021

@TheLostLambda
I think adding this functionality as an optional value would be great.
Are you still interested in that?

@TheLostLambda
Copy link
Member Author

Hi @a-kenji ! Sorry this has sat around for ages! I'm happy to add it as something optional!

Should we make it a config option that allows the user to specify an arbitrary character to use for the corners or just have a rounded_pane_corners boolean toggle?

@a-kenji
Copy link
Contributor

a-kenji commented Jan 4, 2022

@TheLostLambda,

Id say have a boolean toggle for now would be the best option.

@a-kenji
Copy link
Contributor

a-kenji commented Feb 12, 2022

@TheLostLambda,
Just in case this pr is not on your mind anymore a slight ping.

@TheLostLambda
Copy link
Member Author

Closing in favour of #1227 !

@har7an har7an deleted the rounded-corners branch October 23, 2022 15:18
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.

3 participants