rivo / tview

Terminal UI library with rich, interactive widgets — written in Golang
MIT License
11.17k stars 574 forks source link

padding reveals an area without background color #833

Open tcurdt opened 1 year ago

tcurdt commented 1 year ago

Unfortunately setting a padding reveals an area where I cannot set a background color. Here the left and right padding is just white instead of black.

Screenshot 2023-03-26 at 18 19 30

Why isn't the background color being used?

Originally posted by @tcurdt in https://github.com/rivo/tview/issues/831#issuecomment-1484148541

rivo commented 1 year ago

I tried to reproduce this but couldn't. Please post a small code snippet that reproduces this issue.

digitallyserviced commented 1 year ago

@tcurdt Sometimes this can be caused by certain containers not having their base Box primitive's dontClear value set right.

I have had to hack a method to allow this to be modified so that I could ensure that existing, and any custom primitives I developed I could make sure that it cleared the background or not...

My fork which has the method to set this property... https://github.com/digitallyserviced/tview/blob/611a26b9bad195005135a6ce0215280c68d57903/box.go#L136

The current part of the draw function that uses this when filling backgrounds in Box primitive...

why this property is not modifiable externally I am not sure, but is very useful when developing custom primitives

    background := def.Background(b.backgroundColor).Reverse(b.reverse)
    if !b.dontClear {
        for y := b.y; y < b.y+b.height; y++ {
            for x := b.x; x < b.x+b.width; x++ {
                screen.SetContent(x, y, ' ', nil, background)
            }
        }
    }
tcurdt commented 1 year ago

Here is a snippet from the code.

...
    layoutCol2 := tview.NewFlex().
        SetDirection(tview.FlexRow).
        AddItem(identificationTextview, 3, 0, false).
        AddItem(valueInput, 1, 0, false).
        AddItem(detailsTextview, 6, 0, false)
    layoutCol2.SetBorder(true)
    layoutCol2.SetBorderPadding(0, 0, 1, 1)

...

    layoutControl := tview.NewFlex().
        SetDirection(tview.FlexColumn).
        AddItem(layoutCol1, 50, 0, false).
        AddItem(layoutCol2, 23, 0, false).
        AddItem(layoutCol3, 0, 1, false).
        AddItem(layoutCol4, 40, 0, false)

...

Let me know if that already helps. Otherwise I need to try and extract the problem somehow.

digitallyserviced commented 1 year ago

@tcurdt ya your code doesnt really help, because i am already pretty sure of the problem... you need to make dontClear available to your code because Flex is one of the primitives that defines dontClear as true

I do not agree with the idea in the comment for NewFlex stating that you need to define nil flex areas with an empty box to set a bG color... specifically because of what you have experienced without having anything to do with nil flex areas...

// Note that Box, the superclass of Flex, will not clear its contents so that
// any nil flex items will leave their background unchanged. To clear a Flex's
// background before any items are drawn, set it to a box with the desired
// color:
//
func NewFlex() *Flex {
    f := &Flex{
        direction: FlexColumn,
    }
    f.Box = NewBox()
    f.Box.dontClear = true
    return f
}

I already provided the link to what needs to be added to make this modifiable from outside tview https://github.com/digitallyserviced/tview/blob/611a26b9bad195005135a6ce0215280c68d57903/box.go#L136C1-L139C2

digitallyserviced commented 1 year ago

@tcurdt @rivo

860 should resolve this... it definitely should be available outside regardless... it is not just here it becomes an issue. This is why it is included in my fork.

@tcurdt if this PR is merged you would then do something like..

    layoutCol2 := tview.NewFlex().
        SetDirection(tview.FlexRow).
        AddItem(identificationTextview, 3, 0, false).
        AddItem(valueInput, 1, 0, false).
        AddItem(detailsTextview, 6, 0, false)
    layoutCol2.SetBorder(true)
    layoutCol2.SetBorderPadding(0, 0, 1, 1)

    // dontClear disables bg wipes, so set to false for this padding on a flex
    // previous comments show where this is set to `true`
    layoutCol2.SetDontClear(false)
tcurdt commented 1 year ago

@digitallyserviced So far it feels a little unintuitive having to bother with this though.

digitallyserviced commented 1 year ago

@tcurdt

So far it feels a little unintuitive having to bother with this though.

i agree but there are a lot of things in life that are unintitive or bothersome... and you're still alive