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

fix: border size getters when implicit borders are present #411

Merged
merged 5 commits into from
Nov 20, 2024

Conversation

meowgorithm
Copy link
Member

When a border style is set and no borders have been explicitly enabled or disabled, a border is implicitly applied to all sides of the block. Prior to this fix border size getters would incorrectly return 0 when an implicit border is set. This revision corrects that behavior.

In other words:

s := lipgloss.NewStyle.Border(lipgloss.NormalBorder)

// Before
fmt.Println(s.GetBorderLeftSize()) // 0

// After
fmt.Println(s.GetBorderLeftSize()) // 1

@meowgorithm
Copy link
Member Author

@bashbunni Just a heads up about this one. Technically it's a behavioral change but I think it is the right approach here. I plan on merging it today.

I encountered the need for the fix in charmbracelet/bubbletea#1204.

@bashbunni
Copy link
Member

Hey @meowgorithm just a heads up, this is super similar to another open PR https://github.com/charmbracelet/lipgloss/pull/282/files

The main difference being naming + I like your GetBorderXSize() implementations better.

get.go Outdated Show resolved Hide resolved
When a border style is set and no borders have been explicitly enabled
or disabled, a border is implicitly applied to all sides of the
block. Prior to this fix border size getters would incorrectly return
0 when an implicit border is set. This revision corrects that behavior.
get.go Outdated Show resolved Hide resolved
@bashbunni bashbunni merged commit 9b8304f into master Nov 20, 2024
19 checks passed
@bashbunni bashbunni deleted the fix-border-width-getters branch November 20, 2024 16:29
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.

3 participants