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(table): last row out of viewport #731

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

Conversation

System-Glitch
Copy link

When a table cannot display all its rows, the viewport allows scrolling. However, the last row isn't visible.

For example for a table size of 7, the viewport height will be 5 (-2 because of the header). When setting the cursor at position 5, the 6th row should be visible, but isn't. This PR fixes this.

Because cursor is an index (starts at 0), we need to substract 1 to the viewport height to get the correct start position. Otherwise the viewport thinks it can display one more row than it can actually fit when rendering.

Because cursor is an index (starts at 0), we need to substract 1 to the viewport height to get the correct start position.
@System-Glitch
Copy link
Author

Wait before merging this. It looks like it introduces undesirable behavior in some cases.

@System-Glitch System-Glitch marked this pull request as draft February 13, 2025 14:34
@System-Glitch System-Glitch marked this pull request as ready for review February 13, 2025 16:07
@System-Glitch
Copy link
Author

System-Glitch commented Feb 13, 2025

Ready for re-review. I removed unecessary code related to the viewport Y offset. I didn't see any difference after removing it because we only render the relevant rows in UpdateViewport() already.

There is one bug remaining: the selector stays at the bottom when moving up. I tested it on master as well just to be sure I didn't introduce it with these changes. I can confirm it's already happening on master.

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.

2 participants