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

Why inner rectangle of a box is forced moved to screen? #405

Closed
gnojus opened this issue Feb 15, 2020 · 6 comments
Closed

Why inner rectangle of a box is forced moved to screen? #405

gnojus opened this issue Feb 15, 2020 · 6 comments

Comments

@gnojus
Copy link

gnojus commented Feb 15, 2020

Hi.

As I understand from this snippet

tview/box.go

Lines 318 to 323 in ae3d8ca

// Clamp inner rect to screen.
width, height := screen.Size()
if b.innerX < 0 {
b.innerWidth = b.innerX
b.innerX = 0
}

inside Box drawing function the inner rectangle is moved inside the screen. Because of this, displayed content might be different than expected (e. g. when you want to cut part of the text, it will still be visible, just in different position relative to the surrounding box).

This makes scrolling of lets say flex or grid by a single row impossible. Or at least I couldn't figure out a way how to do it.

Would it be possible to add an option to allow out of screen inner rectangles?
Or is it a feature by design and I'm not getting something?

@tslocum
Copy link
Contributor

tslocum commented Feb 15, 2020

I can't speak to why this is clamping occurs. However, if you could further explain what you are trying to achieve I may offer a solution. What do you mean by scrolling a Flex or Grid? Would creating an internal scrolling mechanism by updating a Grid to contain different rows achieve what you are looking for?

@gnojus
Copy link
Author

gnojus commented Feb 15, 2020

Would creating an internal scrolling mechanism by updating a Grid to contain different rows achieve what you are looking for?

Smart, I didn't think about. It's actually doable, but a bit hacky. It would probably require displaying only specific subset of elements instead of everything.

What I want to achieve is basically a multi-line list (scrollable, not necessary selectable), containing multi-line textViews. Similar to a webpage.

And yet changing grid's or flex's position would be much more elegant.

@rivo
Copy link
Owner

rivo commented Feb 19, 2020

Early on, I was thinking of a class Scrollable which may contain another primitive which would only be displayed partially and could be scrolled using keyboard shortcuts. With Grid you do get a similar behaviour but scrolling is then per item, now per row in the terminal.

Such a Scrollable may still be something I'll add in the future but given the backlog I have on here, you may not want to bet on it becoming available very soon.

@gnojus
Copy link
Author

gnojus commented Feb 19, 2020

I see. That Scrollable class would be really useful.
Scrolling per item gives quite different behavior on bigger items, and also cuts box borders: last line sometimes looks like this

└───────────────┘

instead of

├───────────────┤

and therefore gives impression that there isn't anything more to scroll while there are still more items.
And I suppose just allowing opt out clamping isn't a good idea? The problem is that there is no way to achieve that in a custom primitive level, without rewriting the base box.

@rivo rivo closed this as completed in ba670d2 Feb 19, 2020
@rivo
Copy link
Owner

rivo commented Feb 19, 2020

Ok. So the clamping in Box was initially just a way to simply optimize the drawing of primitives. There was no aesthetic reason for it and I agree it actually makes some cases look worse.

I removed it with the latest commit.

This meant that I needed to add screen boundary checking to all primitives. (Theoretically, it's not necessary because tcell ignores writes outside of the screen.) I wanted to have some basic optimization.

Please note that, say, a large TextView may still be slow to scroll because all the reindexing happens independently of the screen dimensions. tview is not yet ready for huge primitives.

@gnojus
Copy link
Author

gnojus commented Feb 19, 2020

Wow that was quick, thanks. Noted about the big buffers.

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

No branches or pull requests

3 participants