-
Notifications
You must be signed in to change notification settings - Fork 230
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
Style.GetFrameSize()
does not account for border when using only BorderStyle()
#281
Open
qualidafial opened this issue
Apr 22, 2024
· 2 comments
· Fixed by Brugarolas/lipgloss#9 · May be fixed by #282
Open
Style.GetFrameSize()
does not account for border when using only BorderStyle()
#281
qualidafial opened this issue
Apr 22, 2024
· 2 comments
· Fixed by Brugarolas/lipgloss#9 · May be fixed by #282
Labels
Comments
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 |
qualidafial
added a commit
to qualidafial/lipgloss
that referenced
this issue
Apr 22, 2024
Return non-zero border sizes when border style is set, and no border sides have been specifically turned on or off. Fixes: charmbracelet#281
Ping! I've submitted PR #282 as a proposed fix for this. Would love your feedback! |
qualidafial
added a commit
to qualidafial/lipgloss
that referenced
this issue
May 7, 2024
Return non-zero border sizes when border style is set, and no border sides have been specifically turned on or off. Fixes: charmbracelet#281
qualidafial
added a commit
to qualidafial/lipgloss
that referenced
this issue
Oct 28, 2024
Return non-zero border sizes when border style is set, and no border sides have been specifically turned on or off. Fixes: charmbracelet#281
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
When rendering, a style configured with
BorderStyle(border)
renders the same as usingBorder(border)
with nosides
arguments, thanks to this check inapplyBorder()
:However, the border sizes reported from
GetHorizontalBorderSize()
,GetVerticalBorderSize()
,GetHorizontalFrameSize()
, andGetVerticalFrameSize()
are not adjusted when usingBorderStyle()
. 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
The final test fails with these errors:
Expected behavior
Given that
BorderStyle(border)
renders the same asBorder(border)
, I would expect methods that calculate border / frame sizes to return the same sizes as when usingBorder()
.This could be resolved in several different ways:
applyBorder
, under the same conditions. This is probably the safest option.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.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.The text was updated successfully, but these errors were encountered: