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 filepanel also filtering main timeline with LL turned on. #716

Merged
merged 2 commits into from
Sep 4, 2018

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Sep 3, 2018

@bwindels bwindels requested a review from a team September 3, 2018 13:04
@ara4n
Copy link
Member

ara4n commented Sep 3, 2018

looks plausible. would be good to get a comment about why this is needed though (as i'm failing to see why/where the timeline filter ever gets written into this object?)

@bwindels
Copy link
Contributor Author

bwindels commented Sep 3, 2018

@ara4n Here: 3363cc4#diff-cf27c1d543e886c89cd9ac8b8aeaf05bR2154

If the method got called once with both the LL & timeline filter (anytime you open the filepanel with LL enabled), the timeline filter would modify the original LL filter object, persisting the filepanel filter on the LL filter.

@ara4n
Copy link
Member

ara4n commented Sep 3, 2018

right. lgtm then - just need to get the comment in the code to spell out where the 'bad' assign happens to avoid someone breaking it or getting confused by it in future.

@bwindels
Copy link
Contributor Author

bwindels commented Sep 3, 2018

Right, will do!

@bwindels
Copy link
Contributor Author

bwindels commented Sep 4, 2018

@ara4n ready for another look

@ara4n
Copy link
Member

ara4n commented Sep 4, 2018

lgtm!

@bwindels bwindels merged commit 3561fd1 into develop Sep 4, 2018
@bwindels bwindels deleted the bwindels/fixfilepanel branch September 4, 2018 15:10
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