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

Fix jumpy behavior when paired with self expanding decorated tasks #4

Merged
merged 4 commits into from
Apr 30, 2018

Conversation

Koji23
Copy link

@Koji23 Koji23 commented Apr 28, 2018

Since the frontend's DecoratedTasks components will sometimes expand themselves on hover we need to expose some way of letting them inform the CellMeasurer Cache and the List component what changes have occurred. Otherwise the expanded tasks will bleed into the task beneath it.

We want to do a deep equals check on the list.rows content before updating the cache/list since the frontend almost always passes in a new lists prop whenever any interaction takes place on the board. (Perhaps due to backbone-redux not using immutability-helpers? RVK uses react-addons-update to similar effect so I doubt the issue is internal) Otherwise all the task lists will clear their caches and resize themselves when any task is moved. It would be nice to fix this in the future however since this is a deep equals check on a user-determined array of task objects.

When doing this deep equals check we want to keep track of the index of the changed rows and not recompute row heights for rows higher than the ones that were changed. Else the list will resize and jump unexpectedly (similar to this bug)

@Koji23 Koji23 requested a review from michelle-becker April 28, 2018 00:15
Copy link

@michelle-becker michelle-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this seems reasonable - certainly works okay. I'm confused why we need the deep equal though - that's kinda expensive if we are doing it for all of the rows...

@Koji23
Copy link
Author

Koji23 commented Apr 30, 2018

Discussed offline about the deep equals check. Agreed that is it expensive but not much worse then the default case of updating every list in all cases. This is an inevitable result of the way the frontend delivers props to the Kanban component. The likely fix for this will be to make better use of immutable data in backbone-redux and then to remove the deep equals check here once that's done

@Koji23 Koji23 merged commit c326987 into master Apr 30, 2018
@Koji23 Koji23 deleted the toolbox branch April 30, 2018 20:11
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