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

widget.Table: show first row/column when navigating with keyboard #5122

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

Conversation

pierrec
Copy link

@pierrec pierrec commented Sep 8, 2024

Fixes #5088.

Description:

When a Table has its header(s) shown, keyboard navigation fails to show the first row or column
if the list was scrolled. This is because the header size is always added to the offset even when
the first row/column is reached. Disabling this for the first row or column fixes the issue.

Fixes #5088

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass.

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

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

Thanks so much for fixing this and adding tests.

Unfortunately it only works for when the ID for row or col is 0.
But also we should ensure the focused cell highlight is visible when the cell index is 1 or above, otherwise the indicator hides behind the header until you reach the first row/col

@pierrec
Copy link
Author

pierrec commented Sep 8, 2024

I tested by moving too fast, I see the issue now. Will get back to it.

pierre added 2 commits September 8, 2024 21:23
The first and last rows may not have been fully put in view
if it was first scrolled a bit and then focused via keyboard.
This change makes sure thatt ScrollTo and RefreshItem are run
even if they are already in focus.
The table size at the focused cell does not need to account for the
header's.

Fixes 5088.
@andydotxyz
Copy link
Member

@pierrec can you look at these failures? Because of the force-push it has lost the build history so it is not possible to see where it broke.
Please do avoid force-push after reviews have started so we get more info on what is changing.

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.

Table Keyboard Navigation with Header Row Hides First Data Row
2 participants