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

Add pagination to admin search #7228

Merged

Conversation

zanturik
Copy link
Contributor

@zanturik zanturik commented Dec 6, 2023

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #7109
Related issues/PRs #7109
License MIT
Documentation PR

What's in this PR?

Add pagination to admin search page

Why?

Because good search pages contains pagination.

Example Usage

Search * in admin search page. Play with pagination.

To Do

@zanturik zanturik changed the base branch from 2.5 to 2.4 December 6, 2023 00:54
@alexander-schranz alexander-schranz added the Feature New functionality not yet included in Sulu label Feb 5, 2024
@alexander-schranz alexander-schranz changed the base branch from 2.4 to 2.6 February 5, 2024 10:24
@alexander-schranz alexander-schranz changed the title Bugfix/add pagination to admin search Add pagination to admin search Feb 5, 2024
@alexander-schranz alexander-schranz added the Design required For this issue a design form our UX expert is required label Mar 18, 2024
@alexander-schranz
Copy link
Member

@zanturik Thx for working on this and sorry for the late response.

I did give this a quick try but it looks like this isn't working. Maybe related to the removed zend lucene part or fasl? But it even not triggers a request so maybe related more to the JS part.

search

The pagination component itself is maybe a little bit small on small screens. So maybe we should make that atleast the same width as the search field.

@alexander-schranz alexander-schranz marked this pull request as draft March 18, 2024 11:28
@zanturik
Copy link
Contributor Author

zanturik commented Mar 18, 2024

I did give this a quick try but it looks like this isn't working.

You are right, my bad, one line was not pushed. Fixed, Please test again.

The pagination component itself is maybe a little bit small on small screens. So maybe we should make that atleast the same width as the search field.

I agree. The problem it that it's a Pagination component displaying results/pagination this way, I'm just reusing it. The problem itself comes from "max-width" of .search-result class. I think it should be fixed in a separate task, since it's not the only place where search is used and design in other places may differ.

@alexander-schranz alexander-schranz removed the Design required For this issue a design form our UX expert is required label Apr 18, 2024
Copy link
Member

@alexander-schranz alexander-schranz left a comment

Choose a reason for hiding this comment

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

I checked this with the Team and the only required change is that the Pagination component should go over the same width as the search field itself. Can you tackle that?

@zanturik
Copy link
Contributor Author

I checked this with the Team and the only required change is that the Pagination component should go over the same width as the search field itself. Can you tackle that?

fixed, please review.

@alexander-schranz alexander-schranz marked this pull request as ready for review April 29, 2024 16:53
Search controller contained the code, needed to be able to run tests of page/limit parameters with TestAdapter.
Fixed, now controller containes only code, that needed to run for the real adapters (Lucene/elastic).
MassiveBundle TestAdapter is ignoring limit and page parameters of search requests, so it's impossible to test them
correctly. Tests adjusted (doesn't try to test limit and page anymore).
@alexander-schranz alexander-schranz force-pushed the bugfix/add-pagination-to-admin-search branch from 978a53d to 734686a Compare April 29, 2024 16:59
@alexander-schranz alexander-schranz enabled auto-merge (squash) April 29, 2024 17:00
@alexander-schranz
Copy link
Member

@zanturik Looks good for me, Thanks you for the contribution! Auto merged when the CI finished.

@alexander-schranz alexander-schranz merged commit 98f886e into sulu:2.6 Apr 29, 2024
8 checks passed
@zanturik zanturik deleted the bugfix/add-pagination-to-admin-search branch May 1, 2024 15:34
NeuralClone added a commit to NeuralClone/sulu that referenced this pull request May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New functionality not yet included in Sulu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

search function in the admin backend only shows 10 results and has no pagination
2 participants