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

padding reveals an area without background color #833

Open
tcurdt opened this issue Apr 6, 2023 · 7 comments
Open

padding reveals an area without background color #833

tcurdt opened this issue Apr 6, 2023 · 7 comments

Comments

@tcurdt
Copy link

tcurdt commented Apr 6, 2023

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 #831 (comment)

@rivo
Copy link
Owner

rivo commented Jun 18, 2023

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

@digitallyserviced
Copy link
Contributor

digitallyserviced commented Jun 25, 2023

@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
Copy link
Author

tcurdt commented Jul 3, 2023

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
Copy link
Contributor

digitallyserviced commented Jul 3, 2023

@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
Copy link
Contributor

digitallyserviced commented Jul 3, 2023

@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
Copy link
Author

tcurdt commented Jul 3, 2023

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

@digitallyserviced
Copy link
Contributor

@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

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

No branches or pull requests

3 participants