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

Crop cells only if memorizedRange is bigger than expected range #94

Merged
merged 6 commits into from
Jul 20, 2021
Merged

Crop cells only if memorizedRange is bigger than expected range #94

merged 6 commits into from
Jul 20, 2021

Conversation

tchudyk
Copy link
Contributor

@tchudyk tchudyk commented Jul 19, 2021

When I added VirtualFlow.visibleCells() listener I'm receiving not ending stream of events, but visible cells list is not changing.
The problem seemed to only affect lists where the cell is higher than the visible area (Maybe related to #80).

VirtualFlow<T, C> view = ...;
//.....
view.visibleCells().addListener(new InvalidationListener() {
    @Override
    public void invalidated(Observable observable) {
        System.out.println("a"); // <--- Not ending stream of events (printing `a`).
    }
});

I think the problem may be in package org.reactfx:reactfx:2.0-M5: MemorizationListImpl.forget(int from, int to)
https://github.com/TomasMikula/ReactFX/blob/537fffdbb2958a77dfbca08b712bb2192862e960/reactfx/src/main/java/org/reactfx/collection/MemoizationList.java#L176

It publish events (memoizedItems.fireRemoveRange(memoChangeFrom, memoRemoved)) even if memoRemoved list is empty.

I'm not sure if this behavior in reactfx is good or wrong, but I found workaround to apply in Flowless in CellListManager.cropTo(...) to forget items only when memorizedRange is bigger then our fromItem-toItem range.

@Jugen
Copy link
Contributor

Jugen commented Jul 20, 2021

Hi @tchudyk thanks for looking into this and the PR, much appreciated :-)

@Jugen Jugen merged commit e746ee2 into FXMisc:master Jul 20, 2021
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.

2 participants