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

Consider Viewport Frame Size when Calculating Visible Lines #337

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Zebbeni
Copy link

@Zebbeni Zebbeni commented Feb 24, 2023

Here's my suggested fix for #336
Basically, the viewport should take into account GetVerticalFrameSize() when calculating visible lines and maxYOffset.

Here's a quick comparison of the fixed behavior with current master, with multiple viewports rendering the same content with different frame formats applied to each. (Screenshots taken after scrolling down as far as possible)

Current Master

current_master

This Branch

with_fix
Note: the behavior of the rightmost viewport is still not ideal- the box should be one character shorter- but the viewport is correctly calling Render() on an empty string, so I think it may be a lipgloss issue.

Example code

package main

import (
	"fmt"
	"os"

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

const (
	viewH   = 8
	viewW   = 14
	content = "line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10"
)

var (
	contentStyle          = lipgloss.NewStyle().Width(viewW)
	borderlessStyle       = lipgloss.NewStyle()
	borderStyle           = borderlessStyle.Copy().Border(lipgloss.RoundedBorder())
	borderPadStyle        = borderStyle.Copy().Padding(1, 1)
	borderPadMargin1Style = borderPadStyle.Copy().Margin(1, 0)
	borderPadMargin2Style = borderPadStyle.Copy().Margin(2, 0)

	headerStyle = lipgloss.NewStyle().Padding(1, 0)

	headers = []string{"simple", "+ border", "+ padding", "+ margin (1)", "+ margin (2)"}
	styles  = []lipgloss.Style{borderlessStyle, borderStyle, borderPadStyle, borderPadMargin1Style, borderPadMargin2Style}
)

type model struct {
	viewports []viewport.Model
}

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

func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	for i := range m.viewports {
		m.viewports[i], _ = m.viewports[i].Update(msg)
	}
	return m, nil
}

func (m *model) View() string {
	names := make([]string, len(headers))
	for i, n := range headers {
		names[i] = fmt.Sprintf("%-14s", n)
	}
	header := lipgloss.JoinHorizontal(lipgloss.Left, names...)
	header = lipgloss.NewStyle().Padding(1, 0).Render(header)

	viewContent := make([]string, len(m.viewports))
	for i, v := range m.viewports {
		viewContent[i] = v.View()
	}
	viewports := lipgloss.JoinHorizontal(lipgloss.Left, viewContent...)

	return lipgloss.JoinVertical(lipgloss.Top, header, viewports)
}

func main() {
	viewports := make([]viewport.Model, len(styles))
	for i, style := range styles {
		v := viewport.New(viewW, viewH)
		v.Style = style
		v.SetContent(contentStyle.Render(content))
		viewports[i] = v
	}

	m := &model{
		viewports: viewports,
	}

	p := tea.NewProgram(m)
	if _, err := p.Run(); err != nil {
		fmt.Println("Run error:", err)
		os.Exit(1)
	}
}

@Zebbeni Zebbeni changed the title Consider viewport frame size when calculating visible lines Consider Viewport Frame Size when Calculating Visible Lines Feb 24, 2023
@maaslalani
Copy link
Member

@Zebbeni Do you mind adding a test to illustrate exactly what this is fixing and to prevent regressions. Looking at the bug / issue I think this makes sense to me.

@Zebbeni
Copy link
Author

Zebbeni commented Mar 17, 2024

@maaslalani No problem! Just added five test cases based on the example code above.

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.

None yet

2 participants