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 #1227

Merged
merged 8 commits into from
Mar 22, 2022
Merged

feat(ui): round frame corners #1227

merged 8 commits into from
Mar 22, 2022

Conversation

TheLostLambda
Copy link
Member

This PR directly replaces #669

Everything now passes around a Style instead of a Palette, letting us add more styling information in the future. For now, it means round corners in themes!

You can now add:

themes:
  default:
    rounded_corners: true

For round corner goodness!
image

As an important BREAKING CHANGE (needs @a-kenji 's input probably), color palettes need to be specified in a subsection now!

To explain using an example, this old theme...

themes:
  tokyo-night-storm:
    fg: [169,177,214] #A9B1D6
    bg: [36,40,59] #24283B
    gray: [86,95,137] #565F89
    black: [65,72,104] #414868
    red: [247,118,142] #F7768E
    green: [158,206,106]  #9ECE6A
    yellow: [224,175,104] #E0AF68
    blue: [122,162,247]  #7AA2F7
    magenta: [187,154,247] #BB9AF7
    cyan: [42,195,222] #2AC3DE
    white: [192,202,245] #C0CAF5
    orange: [255,158,100] #FF9E64

Would become:

themes:
  tokyo-night-storm:
    palette:
      fg: [169,177,214] #A9B1D6
      bg: [36,40,59] #24283B
      gray: [86,95,137] #565F89
      black: [65,72,104] #414868
      red: [247,118,142] #F7768E
      green: [158,206,106]  #9ECE6A
      yellow: [224,175,104] #E0AF68
      blue: [122,162,247]  #7AA2F7
      magenta: [187,154,247] #BB9AF7
      cyan: [42,195,222] #2AC3DE
      white: [192,202,245] #C0CAF5
      orange: [255,158,100] #FF9E64

With the optional corner parameter:

themes:
  tokyo-night-storm:
    rounded_corners: true
    palette:
      fg: [169,177,214] #A9B1D6
      bg: [36,40,59] #24283B
      gray: [86,95,137] #565F89
      black: [65,72,104] #414868
      red: [247,118,142] #F7768E
      green: [158,206,106]  #9ECE6A
      yellow: [224,175,104] #E0AF68
      blue: [122,162,247]  #7AA2F7
      magenta: [187,154,247] #BB9AF7
      cyan: [42,195,222] #2AC3DE
      white: [192,202,245] #C0CAF5
      orange: [255,158,100] #FF9E64

@TheLostLambda TheLostLambda temporarily deployed to cachix March 16, 2022 23:18 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 16, 2022 23:18 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 16, 2022 23:18 Inactive
@a-kenji
Copy link
Contributor

a-kenji commented Mar 16, 2022

Yay. This is amazing!

I do think the configuration should be:

pane_frames:
    rounded_corners: true

so we can build upon that and go with something like #680:

pane_frames:
  rounded_corners: true
  top:
    left: [title, ]
    right: [scroll, ]
  bottom:
    left: []
    right: []

@TheLostLambda
Copy link
Member Author

Yay. This is amazing!

I do think the configuration should be:

pane_frames:
    rounded_corners: true

so we can build upon that and go with something like #680:

pane_frames:
  rounded_corners: true
  top:
    left: [title, ]
    right: [scroll, ]
  bottom:
    left: []
    right: []

A good idea! Just to clarify, is the pane_frames nested in the themes part of the config still?

@a-kenji
Copy link
Contributor

a-kenji commented Mar 16, 2022

That is a good question, I would say no, since it should mostly handle functionality.
And if someone wanted to load a specific theme / pane_frame combination, they could always load a layout.

@TheLostLambda
Copy link
Member Author

Okay, I can do that! Just making sure you'd like to add that to this main config? It feels a little bit like the sort of thing that would belong in a theme and pane_frames feels a bit niche for the top-level, but if you don't think things like the title and scroll should be in a theme, then that's alright and we should keep pane things together.
image

@a-kenji
Copy link
Contributor

a-kenji commented Mar 17, 2022

You are right, it does feel out of place in the main config. But I do think it seems very unwieldy under the theme section. Since you would need to configure it for every color scheme separately. Maybe put it under something like ui ?

@TheLostLambda
Copy link
Member Author

You are right, it does feel out of place in the main config. But I do think it seems very unwieldy under the theme section. Since you would need to configure it for every color scheme separately. Maybe put it under something like ui ?

I'd be happy with that, something like ui -> pane_frames -> rounded_corners then?

@a-kenji
Copy link
Contributor

a-kenji commented Mar 17, 2022

Yes, that sounds very good!

@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:15 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:15 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:15 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:15 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:49 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:49 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:49 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 19, 2022 23:50 Inactive
@TheLostLambda
Copy link
Member Author

Hopefully I'll wrap up this PR – got caught up with loads of merge and cleanup work tonight!

@TheLostLambda TheLostLambda temporarily deployed to cachix March 21, 2022 22:08 Inactive
@TheLostLambda TheLostLambda temporarily deployed to cachix March 21, 2022 22:08 Inactive
@TheLostLambda
Copy link
Member Author

@a-kenji Took me long enough... This should be ready for final review!

@TheLostLambda TheLostLambda temporarily deployed to cachix March 22, 2022 00:15 Inactive
@TheLostLambda
Copy link
Member Author

Oh, and for clarity, the config option is now:

ui:
    pane_frames:
        rounded_corners: true

Copy link
Contributor

@a-kenji a-kenji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT, thank you! This is awesome.

I personally would call it round_corners instead of rounded_corners,
but I am not a native speaker.

@TheLostLambda
Copy link
Member Author

I might stick with rounded corners for now, but they do seem pretty interchangeable nowadays!
image

@TheLostLambda TheLostLambda merged commit 9bfafde into main Mar 22, 2022
@imsnif
Copy link
Member

imsnif commented Mar 22, 2022

Do we want to add docs for this?

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