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

filter filetypes for latest sermon view #41

Closed
wants to merge 1 commit into
base: master
from

Conversation

2 participants
@fri-sch
Contributor

fri-sch commented Feb 3, 2017

Hi Thomas,

I'm using the "latest sermon" menu option and found, that the filetype filter does not work in this case.
I want to link to the latest sermon, that also has an audio file attached (ignore sermons without audio).
I enabled the option with this fix. Now I can set the filetype filter to 'audio' and get what I want.
I thought you might want to pull this so I prepared a pull request for you.

Grüße
Frieder

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Feb 5, 2017

Owner

I think this needs a bit of improvement so it uses states instead of directly reading the params. Other than that it looks fine to me. Also need some time to test it.

Owner

Bakual commented Feb 5, 2017

I think this needs a bit of improvement so it uses states instead of directly reading the params. Other than that it looks fine to me. Also need some time to test it.

@fri-sch

This comment has been minimized.

Show comment
Hide comment
@fri-sch

fri-sch Feb 5, 2017

Contributor

I tried to implement it using states first but calling getState() inside getLatest() causes populateState() to be called too early (when id is not yet set). This is why I read from the params directly.
But I'm far from being an expert, so there might be better ways to do this.

Contributor

fri-sch commented Feb 5, 2017

I tried to implement it using states first but calling getState() inside getLatest() causes populateState() to be called too early (when id is not yet set). This is why I read from the params directly.
But I'm far from being an expert, so there might be better ways to do this.

@Bakual

This comment has been minimized.

Show comment
Hide comment
@Bakual

Bakual Feb 11, 2017

Owner

I've cherrypicked your commit and improved it so it uses states now. Also did some other improvements on the way.

Thanks!

Owner

Bakual commented Feb 11, 2017

I've cherrypicked your commit and improved it so it uses states now. Also did some other improvements on the way.

Thanks!

@Bakual Bakual closed this Feb 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment