Schedule rebuild immediately on scroll to avoid flicker. #65
Conversation
Use visibility observers instead of guessing at when visibility changes. Fixes bug when initially only 4 rows were drawn for the memory view.
| @@ -13,7 +14,7 @@ import 'utils.dart'; | |||
|
|
|||
| // TODO(devoncarew): fixed position header | |||
|
|
|||
| class Table<T> extends Object with SetStateMixin, OnAddedToDomMixin { | |||
| class Table<T> extends Object with SetStateMixin { | |||
There was a problem hiding this comment.
I believe this was the only use of OnAddedToDomMixin; we should be able to remove that class (and CoreElementOwner as well?).
There was a problem hiding this comment.
Removed. @DanTup feel free to add back OnAddedToDomMixin if you find a case where it is needed. I expect we can manually trigger some dispose calls if there are some lifecycle cases we really care about.
There was a problem hiding this comment.
Neat, I didn't know about this. If it all works the same, this LGTM (I think I added tests for this, but not sure if they run on CI, so may need to be run manually if you didn't already).
It seems like current stable Safari doesn't support it - though tech preview does, so probably not a concern (I don't know what the intention is for browser support here).
lib/tables.dart
Outdated
| @@ -28,7 +29,8 @@ class Table<T> extends Object with SetStateMixin { | |||
| _spacerBeforeVisibleRows = new CoreElement('tr'); | |||
| _spacerAfterVisibleRows = new CoreElement('tr'); | |||
|
|
|||
| _visibilityObserver = new IntersectionObserver(_visibilityChange); | |||
| // TODO(jacobr): remove call to allowInterop once bug in dart:html is fixed. | |||
There was a problem hiding this comment.
Is there a GitHub issue for this bug? If so, adding it in a comment would make it easier to check when it can be removed.
There was a problem hiding this comment.
For browser support I think we only need to support chrome as we will generally be launching this app directly from the command line. Added
#66
to remind users if they open the devtools with an unsupported browser.
Filed
dart-lang/sdk#35484
to fix dart:html and referenced that bug in my comment.
There was a problem hiding this comment.
The tests for this do run on travis as they broke without the allowInterop call.
There was a problem hiding this comment.
Ah, cool - I couldn't remember which ones we'd got working :-)
Use visibility observers instead of guessing at when visibility changes.
Visibility observers are an almost magic bullet for infinite scrolling on the web so I was able to strip out the mutation observer and window sizing code.
Fixes bug when initially only 4 rows were drawn for the memory view on first load.
Fixes
#64
Fixes flicker part of
#25