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

Alternative QueryController approach #189

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bdunogier
Copy link
Member

Applies the concepts from ezsystems/ezpublish-kernel#1473

Updates the blog view to use the QueryController.

@bdunogier bdunogier force-pushed the ezp24624-query_controller_take2 branch 2 times, most recently from 2abfc5c to df232e8 Compare October 18, 2015 07:16
@clash82
Copy link
Contributor

clash82 commented Oct 19, 2015

Awesome! can't wait to see this in action with pagination support :)

@andrerom
Copy link
Contributor

Any POV on how we should deal with Paging? Got feedback that PagerFanta is not really good compared to what we had in legacy, which was purely template based. But unsure on which reason that would be.

@crevillo
Copy link
Contributor

@andrerom Probably PagerFanta can require more work if you want to theme your paginations, but other than that, i'm really happy with it :)
Edit: For one of our sites we did a custom Pagination extending from one of those offered by PagerFanta. Probably could be an option create an eZPublishPaginator like it...

@bdunogier
Copy link
Member Author

I don't think we can really skip some sort of support for a Pager here.

The fact that PagerFanta might not be right for everybody can be an issue. One way would be to make that extensible to some extent. It shouldn't be very complicated: the code that builds the actual result variable based on the search results can be refactored into an external service. We must support both pager & not pager (as it is, semantically, a QueryController option), and we should make sure that the pager implementation can easily be swapped with another one, by means of service container aliasing.

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

Successfully merging this pull request may close these issues.

4 participants