lipgloss
lipgloss copied to clipboard
`Style.GetFrameSize()` does not account for border when using only `BorderStyle()`
Describe the bug
When rendering, a style configured with BorderStyle(border)
renders the same as using Border(border)
with no sides
arguments, thanks to this check in applyBorder()
:
if border != noBorder && !(topSet || rightSet || bottomSet || leftSet) {
hasTop = true
hasRight = true
hasBottom = true
hasLeft = true
}
However, the border sizes reported from GetHorizontalBorderSize()
, GetVerticalBorderSize()
, GetHorizontalFrameSize()
, and GetVerticalFrameSize()
are not adjusted when using BorderStyle()
. I'm new to the Bubbletea ecosystem and have been trying to use these functions to calculate content sizes, and kept getting frames that exceeded the intended size without understanding why.
To Reproduce
package lipgloss
import "testing"
func TestStyle_GetBorderSizes(t *testing.T) {
tests := []struct {
name string
style Style
wantX int
wantY int
}{
{
name: "Default style",
style: NewStyle(),
wantX: 0,
wantY: 0,
},
{
name: "Border(NormalBorder())",
style: NewStyle().Border(NormalBorder()),
wantX: 2,
wantY: 2,
},
{
name: "Border(NormalBorder(), true)",
style: NewStyle().Border(NormalBorder(), true),
wantX: 2,
wantY: 2,
},
{
name: "Border(NormalBorder(), true, false)",
style: NewStyle().Border(NormalBorder(), true, false),
wantX: 0,
wantY: 2,
},
{
name: "Border(NormalBorder(), true, true, false)",
style: NewStyle().Border(NormalBorder(), true, true, false),
wantX: 2,
wantY: 1,
},
{
name: "Border(NormalBorder(), true, true, false, false)",
style: NewStyle().Border(NormalBorder(), true, true, false, false),
wantX: 1,
wantY: 1,
},
{
name: "BorderTop(true).BorderStyle(NormalBorder())",
style: NewStyle().BorderTop(true).BorderStyle(NormalBorder()),
wantX: 0,
wantY: 1,
},
{
name: "BorderStyle(NormalBorder())",
style: NewStyle().BorderStyle(NormalBorder()),
wantX: 2,
wantY: 2,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotX := tt.style.GetHorizontalBorderSize()
if gotX != tt.wantX {
t.Errorf("Style.GetHorizontalBorderSize() got %d, want %d", gotX, tt.wantX)
}
gotY := tt.style.GetVerticalBorderSize()
if gotY != tt.wantY {
t.Errorf("Style.GetVerticalBorderSize() got %d, want %d", gotY, tt.wantY)
}
gotX = tt.style.GetHorizontalFrameSize()
if gotX != tt.wantX {
t.Errorf("Style.GetHorizontalFrameSize() got %d, want %d", gotX, tt.wantX)
}
gotY = tt.style.GetVerticalFrameSize()
if gotY != tt.wantY {
t.Errorf("Style.GetVerticalFrameSize() got %d, want %d", gotY, tt.wantY)
}
gotX, gotY = tt.style.GetFrameSize()
if gotX != tt.wantX || gotY != tt.wantY {
t.Errorf("Style.GetFrameSize() got (%d, %d), want (%d, %d)", gotX, gotY, tt.wantX, tt.wantY)
}
})
}
}
The final test fails with these errors:
=== RUN TestStyle_GetBorderSizes/BorderStyle(NormalBorder())
borders_test.go:66: Style.GetHorizontalBorderSize() got 0, want 2
borders_test.go:71: Style.GetVerticalBorderSize() got 0, want 2
borders_test.go:76: Style.GetHorizontalFrameSize() got 0, want 2
borders_test.go:81: Style.GetVerticalFrameSize() got 0, want 2
borders_test.go:86: Style.GetFrameSize() got (0, 0), want (2, 2)
--- FAIL: TestStyle_GetBorderSizes/BorderStyle(NormalBorder()) (0.00s)
Expected behavior
Given that BorderStyle(border)
renders the same as Border(border)
, I would expect methods that calculate border / frame sizes to return the same sizes as when using Border()
.
This could be resolved in several different ways:
- Perform an on-the-fly calculation as is being done in
applyBorder
, under the same conditions. This is probably the safest option. - Have
BorderStyle
explicitly set all border sides to visible if none of the border side keys have been set yet. This might be risky depending on whether somebody is using inherited styles, and in what order they set their style properties. - Invert the default visibility in the methods that calculate border sizes of each border, e.g.:
func (s Style) GetBorderLeftSize() int {
if !s.getAsBool(borderLeftKey, true) { // <- currently this uses a default of false.
return 0
}
return s.getBorderStyle().GetLeftSize()
}
This last approach could also risk changing behavior for existing applications, if they are relying on explicit calls like BorderLeft(true)
to turn on border sizes.
I'm happy to submit a PR for this if the team has a preference on which solution they prefer.
I personally favor computing these on the fly in the Get<Horizontal|Vertical><Border|Frame>Size()
methods as it seems to agree with how Render
enables the border on the fly.
Ping! I've submitted PR #282 as a proposed fix for this. Would love your feedback!