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] fix: re-render immediately on changes in batched mode #5284

Merged
merged 5 commits into from
May 4, 2022

Conversation

rynorris
Copy link
Contributor

@rynorris rynorris commented May 4, 2022

Fixes #5193, fixes #5253

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

When TableBodyCells props change, it resets the batcher, but does so in the post-render hook, and doesn't trigger another render. This means that the batcher never gets a chance to run again.

On the following time the component is rendered, it will "notice" the empty batcher and render all the cells again. This is why the update works ever second time you change something.

Reviewers should focus on:

  • Is it safe to just throw a forceUpdate() here? Or is there a better way to trigger a re-render in this case?

Testing

In table-dev-app I added the cell content to the cellRendererDependencies. You can see that if you change the cell content, the table only re-renders every 2 times if you comment out the new forceUpdate(). With the forceUpdate() it re-renders every time as expected.

Screenshot

Before

2022-05-04 08 57 58

After

2022-05-04 08 59 04

@adidahiya
Copy link
Contributor

preview build: table-dev-app, docs-app

@adidahiya
Copy link
Contributor

I confirmed that this fixes #5253 as well, you can see that in the preview build for the latest commit. I also added documentation for cellRendererDependencies.

@adidahiya adidahiya changed the title Fix batched table re-render [table] fix: re-render immediately on changes in batched mode May 4, 2022
@adidahiya adidahiya merged commit 0ad215a into palantir:develop May 4, 2022
adidahiya added a commit that referenced this pull request May 4, 2022
Co-authored-by: Adi Dahiya <adahiya@palantir.com>
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 sorting doesn't re-render Batched update table only re-renders once every 2 updates
2 participants