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

[WIP] feat: better borders in tabs/layout #239

Merged
merged 4 commits into from
Feb 10, 2025
Merged

Conversation

mierak
Copy link
Owner

@mierak mierak commented Feb 4, 2025

This PR deprecates the border_type property on tabs, it will be silently ignored. Instead you can now configure the borders per Pane/Split which allows you to replicate the former behavior while providing more control over them at the same time.

The borders are specified as borders: "<value>" where value can be NONE, ALL, RIGHT, LEFT, TOP, BOTTOM, their combination, ie. borders: "RIGHT | LEFT" or omitted altogether which implies NONE.

Note that you now have to account for the borders if you use exact sizing of panes. For example if your Header is supposed to be 2 rows tall and has borders at TOP and BOTTOM you must set its size as 4 as seen in the example below.

Consider this layout:

Click to expand
layout: Split(
    direction: Vertical,
    panes: [
        (
            borders: "ALL",
            pane: Pane(Header),
            size: "4",
        ),
        (
            pane: Pane(Tabs),
            size: "3",
        ),
        (
            borders: "ALL",
            pane: Pane(TabContent),
            size: "100%",
        ),
        (
            size: "3",
            pane: Split(
                direction: Horizontal,
                panes: [
                    (
                        borders: "ALL",
                        pane: Pane(ProgressBar),
                        size: "100%",
                    ),
                ]
            ),
        ),
    ],
),

Along with this tab definition:

Click to expand
(
    name: "Queue",
    pane: Split(
        direction: Horizontal,
        panes: [
            (
                size: "40%",
                borders: "RIGHT",
                pane: Pane(AlbumArt),
            ),
            (
                size: "60%",
                pane: Pane(Queue),
            ),
        ],
    ),
),

Produces this result:

image

This was referenced Feb 4, 2025
@mierak mierak force-pushed the feat/better-borders branch from fcdfe3f to c091220 Compare February 6, 2025 23:46
@mierak mierak marked this pull request as ready for review February 10, 2025 20:16
@mierak
Copy link
Owner Author

mierak commented Feb 10, 2025

There are no issues in my testing, please open a new issue if you find some!

@mierak mierak merged commit 9a686e6 into master Feb 10, 2025
7 checks passed
@mierak mierak deleted the feat/better-borders branch February 10, 2025 20:33
@soifou
Copy link
Contributor

soifou commented Feb 12, 2025

Nice refactor, I adapted my config using this new layout and, so far, this works perfectly, thanks!

Some thoughts about this subject:

  1. I've always found draw_borders to be somewhat misleading since it only adds borders to certain parts of the UI — specifically around tabs and the vertical separators in the dirstack. The documentation for this setting is also quite vague. Now that you can achieve more or less the same effect for tabs using borders, the only missing feature is the ability to manage borders for the dirstack. Why not assume that if TabContent has full borders, the dirstack should have separators as well? This way, we could deprecate the draw_borders setting, or perhaps repurpose it to serve its original semantic purpose: toggling overall borders on or off.
  2. Allowing users to manage borders as they like is great, but wouldn’t it be even better to support border styles as well? For example, popups use rounded borders, while borders: "ALL" implies solid borders. This creates a mismatch in the overall style.
  3. It might be worth mentioning in the doc that tweaking borders requires adjusting sizes accordingly. For instance, combining borders: ALL with size: 2 will only draw borders, hiding the content.

@mierak
Copy link
Owner Author

mierak commented Feb 12, 2025

I've always found draw_borders to be somewhat misleading

I dont like it either and was planning on deprecating it altogether, but I kinda like both your suggestions. I will sit on this one for a bit.

Allowing users to manage borders as they like is great, but wouldn’t it be even better to support border styles as well? For example, popups use rounded borders, while borders: "ALL" implies solid borders. This creates a mismatch in the overall style.

The biggest issue I have here is that I would like to be able to have connected borders (for example if you create a grid layout with single inner borders, now there are gaps between them). There is an issue opened in ratatui for that to happen automagically ratatui/ratatui#1468 so I am sort of waiting for whats going to happen there, because I don't know how the different border styles will interact. But I definitely want to introduce border style config as well.

It might be worth mentioning in the doc that tweaking borders requires adjusting sizes accordingly. For instance, combining borders: ALL with size: 2 will only draw borders, hiding the content.

I swear I had it in there 😄, I will take a look

Thanks for the suggestions as always!

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.

2 participants