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

Make filter aware of changes in cache (11794) #3

Merged
merged 3 commits into from Sep 21, 2016
Merged

Conversation

colemancda
Copy link
Contributor

Currently filter are statically created and shared across view controllers that use it; not being aware of data changes in the cache.
Say we get a data update adding one event to a track, that track won't show in filters.
We need to make filters aware of changes in model data.

Copy link
Member

@gcutrini gcutrini left a comment

Choose a reason for hiding this comment

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

Clearing all active filters will not set didChangeActiveTalks, thus automatically reenabling the Hide Past Talks filter on next timer update.


fallthrough

case .ActiveTalks:

FilterManager.shared.filter.value.didChangeActiveTalks = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gcutrini Here is am setting didChangeActiveTalks to true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gcutrini Are you seeing an issue?

Clearing all active filters will not set didChangeActiveTalks, thus automatically reenabling the Hide Past Talks filter on next timer update.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, not on cell's toggle selection, but when clearing filters from event view controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I didn't think of that xD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, pls test and approve

@gcutrini gcutrini merged commit 0c79e56 into develop Sep 21, 2016
@gcutrini
Copy link
Member

Merged! Great job!

@gcutrini gcutrini deleted the feature/11794 branch September 21, 2016 15:22
@colemancda colemancda restored the feature/11794 branch September 21, 2016 15:22
@colemancda colemancda deleted the feature/11794 branch September 21, 2016 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants