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

Table rows render outside of the visible viewport when table reattaches to page #1753

Closed
mollykreis opened this issue Jan 16, 2024 · 0 comments · Fixed by #1766
Closed

Table rows render outside of the visible viewport when table reattaches to page #1753

mollykreis opened this issue Jan 16, 2024 · 0 comments · Fixed by #1766
Assignees
Labels
bug Something isn't working

Comments

@mollykreis
Copy link
Contributor

🐛 Bug Report

When the nimble-table reattaches to a page after being detached, the virtualizer and table scroll position are reloaded out of sync with each other. As a result, the table's rows don't render with the correct Y position in the table's viewport, causing either no rows to be visible or for there to be blank space between the header and the first rendered row. Interacting with the table's scrollbar fixes the problem.

image

💻 Repro or Code Sample

This issue was originally reported within SLE, so there are ways to get into this situation in an application. However, I was able to reproduce the problem within storybook by interacting with the DOM in dev tools.

  1. Go to the nimble-table's storybook page with a large data set
  2. Scroll down in the table (any amount)
  3. In dev tools, assign the nimble-table element to a variable (say tableElement)
  4. Using dev tools, delete the table from the DOM
  5. Readd the table to the DOM (e.g. call appendChild(tableElement) on the nimble-theme-provider element in the storybook story)
  6. Notice that the table rows don't render correctly. Depending on how much you scrolled down, either none or only some of the table's rows will render.

🤔 Expected Behavior

When the table is reattached, you should be able to see its rows.

😯 Current Behavior

When the table is reattached, its viewport is not filled with rows (though they do exist in the DOM).

💁 Possible Solution

The table and virtualizer's connectedCallback functions need to handle the case where the table is being reattached rather than attached for the first time. Specifically, this means that the virtualizer's rowContainerYOffset and the scroll position of the table's viewport need to be reloaded in a consistent state.

🔦 Context

In SLE, this is causing tables to load incorrectly after either hitting the "Back" button in the browser or when switching anchor-tab pages.

🌍 Your Environment

Does not appear to be browser/OS specific.

@mollykreis mollykreis added bug Something isn't working triage New issue that needs to be reviewed labels Jan 16, 2024
@mollykreis mollykreis changed the title Table rows render outside of the viewport when table reattaches to page Table rows render outside of the visible viewport when table reattaches to page Jan 16, 2024
@mollykreis mollykreis self-assigned this Jan 19, 2024
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Jan 23, 2024
rajsite added a commit that referenced this issue Jan 24, 2024
… reattached to the DOM (#1766)

# Pull Request

## 🤨 Rationale

Fixes #1753 

## 👩‍💻 Implementation

The bug in the existing code is that the table's viewport always reset
to a scroll position of 0 when reconnected, but the virutalizer
maintained its state of being scrolled to a certain position. To fix
this problem, I updated the table's viewport scroll position to match
the virtualizer's scroll position once the state of the virtualizer has
been updated upon reconnection of the `nimble-table` element.

## 🧪 Testing

- Manually tested
- Wrote some new unit tests that verify that the scroll position and
rendered rows are as expected when the table is disconnected and
reconnected. This includes tests that update the data while the table is
not connected.

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [ ] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <rajsite@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
2 participants