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

feat: introduce vertical scrollbar #536

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nervo
Copy link
Contributor

@nervo nervo commented Jun 7, 2024

Well, here is a pull request on a component i really found missing in the bubbletea ecosystem: a scrollbar \o/

Only a vertical one, available as a model, is provided:

sb := scrollbar.NewVertical()

It is easily customizable with styles

sb.Style = sb.Style.Border(lipgloss.RoundedBorder(), true) // Beautiful rounded borders around the bar
sb.ThumbStyle = sb.ThumbStyle.SetString("≡") // Change thumb apperance

It can updated by dedicated messages...

m.scrollbar, cmd = m.scrollbar.Update(scrollbar.HeightMsg(12))
m.scrollbar, cmd = m.scrollbar.Update(scrollbar.Msg{
	Total:   test.total,
	Visible: test.visible,
	Offset:  test.offset,
})

...or directly linked to a viewport:

m.scrollbar, cmd = m.scrollbar.Update(viewport.Height)
m.scrollbar, cmd = m.scrollbar.Update(viewport) // Get the total, visible and offset parameters from viewport

If you're interested, feel free to review, comment, etc... ❤️

That being said, there are several points on which I would like advice:

  1. Do i have to make the scrollbar struct a real model (Update method returns a tea.Model) or an hybrid one (Update method returns the struct itself) ?
  2. Is it better to provide some options to the constructor ? something like:
sb := scrollbar.NewVertical(scrollbar.WithStyle(...), scrollbar.WithThumbStyle(...), ...)
  1. Are you ok with the scrollbar/track/thumb wording ? It's actually inspired by browsers wording (see: https://developer.chrome.com/docs/css-ui/scrollbar-styling)
  2. I really think mouse integration could be a killer feature, but i'm not sure how to (properly) handle it in a portable manner...
  3. I'm experiencing screen blinkings with vhs (see example.gif). Is there something i can do ?

huh, and I almost forgot the demo :)

example

@meowgorithm
Copy link
Member

This is so nice. It may take us a moment to get to this one, but it's on our radar. In the meantime do you mind fixing the lint stuff?

@nervo
Copy link
Contributor Author

nervo commented Jun 19, 2024

@meowgorithm rebased, and lint fixed 👍


func (m model) View() string {
if m.viewport.TotalLineCount() > m.viewport.VisibleLineCount() {
return lipgloss.JoinHorizontal(lipgloss.Left,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use lipgloss.Top rather than lipgloss.Left

@caarlos0 caarlos0 added enhancement New feature or request and removed triage labels Aug 13, 2024
@meowgorithm
Copy link
Member

meowgorithm commented Aug 22, 2024

Hey again, @nervo and thanks (again) for waiting on this. This is SO awesome. We're really grateful for the PR!

My first thought was whether this should just be part of viewport but I do think it's more flexible having it separated. We'll look at the API a little closer but generally I think this one is pretty close.

Per your questions:

  1. Do i have to make the scrollbar struct a real model (Update method returns a tea.Model) or an hybrid one (Update method returns the struct itself) ?

No, you're doing the right thing here. These are really UI fragments and not full models, and these things get fussier to work with when they're true models.

  1. Is it better to provide some options to the constructor ? something like:
sb := scrollbar.NewVertical(scrollbar.WithStyle(...), scrollbar.WithThumbStyle(...), ...)

It doesn't matter too much, but it's a nice detail. Up to you.

  1. Are you ok with the scrollbar/track/thumb wording ? It's actually inspired by browsers wording (see: https://developer.chrome.com/docs/css-ui/scrollbar-styling)

Yes, totally. The naming here is spot on.

  1. I really think mouse integration could be a killer feature, but i'm not sure how to (properly) handle it in a portable manner...

There are scroll wheel events available you can take advantage of if the mouse is enabled.

  1. I'm experiencing screen blinkings with vhs (see example.gif). Is there something i can do ?

There are a few internal factors contributing to this, most of which should be solved for soon. But in the meantime, since you included the tape file, we can clean this up for you when this is merged.

@bashbunni
Copy link
Member

I think it's nice having this separate from viewport in case people want to use this with a table or their own bubbles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants