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(viewport): pad width to contentWidth #388

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

ivanvc
Copy link
Contributor

@ivanvc ivanvc commented Jun 15, 2023

I have an application that uses the viewport, and I added borders to the viewport. I noticed the width is not set to what can be set through .Width(), as I'm loading variable width content, which shrinks and grows depending on what it currently renders.

My initial workaround was to force the style from the viewport to set a fixed width. However, that didn't work. Then, I added spaces to the last line of the content to match the viewport's width, which worked, but I believe the end-user code shouldn't be handling this case.

So, with this PR, I'm padding the width of the viewport content to what has been specified in its width. Please look at the screenshots from a minimal application reproducing the issue.

Before:
Screenshot from 2023-06-15 10-18-06

After:
Screenshot from 2023-06-15 10-12-56

And this is the code that reproduces the issue:

package main

// An example program demonstrating the pager component from the Bubbles
// component library.

import (
	"fmt"
	"os"

	"github.com/charmbracelet/bubbles/viewport"
	tea "github.com/charmbracelet/bubbletea"
	"github.com/charmbracelet/lipgloss"
)

type model struct {
	ready    bool
	viewport viewport.Model
}

func (m model) Init() tea.Cmd {
	return nil
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	switch msg := msg.(type) {
	case tea.KeyMsg:
		if msg.String() == "ctrl+c" {
			return m, tea.Quit
		}

	case tea.WindowSizeMsg:
		if !m.ready {
			m.viewport = viewport.New(msg.Width, msg.Height)
			m.viewport.Style = lipgloss.NewStyle().BorderStyle(lipgloss.NormalBorder()).BorderForeground(lipgloss.Color("4")).Padding(0, 1)
			m.viewport.SetContent("testing")
			m.ready = true
		} else {
			m.viewport.Width = msg.Width
			m.viewport.Height = msg.Height
		}
	}

	var cmd tea.Cmd
	m.viewport, cmd = m.viewport.Update(msg)

	return m, cmd
}

func (m model) View() string {
	if !m.ready {
		return "\n  Initializing..."
	}
	return m.viewport.View()
}

func main() {
	p := tea.NewProgram(
		new(model),
		tea.WithAltScreen(),
	)

	if _, err := p.Run(); err != nil {
		fmt.Println("could not run program:", err)
		os.Exit(1)
	}
}

@holt-crews
Copy link

I just ran into the same issue and was going to suggest the same change. Any update on this @ivanvc? Did you find a fix/workaround or are you still waiting for this pr to get merged?

@maaslalani maaslalani merged commit 1ba1200 into charmbracelet:master Jan 8, 2024
@maaslalani
Copy link
Contributor

Thank you so much for the PR @ivanvc, great catch with this bug! Sorry for the delay in merging!

@ivanvc ivanvc deleted the fix-viewport-width branch January 19, 2024 23:08
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