Page MenuHomePhabricator

Don't use a custom keyMapper
ClosedPublic

Authored by max on Jul 28 2019, 9:08 PM.

Details

Reviewers
schrockn
bengotow
Group Reviewers
Restricted Project
Commits
R1:b568e44bed11: Don't use a custom keyMapper
Summary
Test Plan

Manual

Diff Detail

Repository
R1 dagster
Branch
fix-row-height-overflow
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

How did you verify that this is a perf win?

The opposite, I think this might make table render performance worse; I haven't measured.

This revision is now accepted and ready to land.Jul 29 2019, 3:12 AM

Filtering breaks this apparently

This revision now requires changes to proceed.Jul 29 2019, 3:16 AM

Resolve issue with filtering nodes

This revision is now accepted and ready to land.Jul 29 2019, 4:25 AM

Looks good to me! I think since we're blowing away and re-creating the cache now when the node set changes, we don't need the custom key mapping (you're correct that it was allowing you to filter the list without recomputing the height of the rows).

This revision was automatically updated to reflect the committed changes.