An id should be unique, but we have scan-index appear twice on the scan index page. To test: * Go to advanced search * Select Author and add author name * Check checkbox for scan index * Search * Verify in the source code that the id is doubled up
Created attachment 149578 [details] [review] Bug 33505: Improve styling of scan index page This patch makes some changes to the 'scan index' page. * Updates second h1 heading "Scan index:" to h2 level heading "Scan index" * Removes table from the search form * Moves search index pull down and search term into one line * Reorders options with search index first and search term after, matching the advanced search form * Removes the catalog results toolbar from the top, card hold etc. are not needed here. * Fixes doubled up id attribute: scan-index by turning one into scan-index-term. To test: * Go to advanced search in staff interface * Activate 'More options' * Check checkbox for scan indexes * Switch to author (especially when using Elasticsearch) * Search for C * Check display, especially search options, options at the top of the page etc. * Apply patch * Verify changes listed above * Regression test the normal search results, all should look the same with and without patch. * If you think the scan index page is nicer now - sign off.
Created attachment 149579 [details] [review] Bug 33505: Improve styling of scan index page This patch makes some changes to the 'scan index' page. * Updates second h1 heading "Scan index:" to h2 level heading "Scan index" * Removes table from the search form * Moves search index pull down and search term into one line * Reorders options with search index first and search term after, matching the advanced search form * Removes the catalog results toolbar from the top, card hold etc. are not needed here. * Fixes doubled up id attribute: scan-index by turning one into scan-index-term. To test: * Go to advanced search in staff interface * Activate 'More options' * Check checkbox for scan indexes * Switch to author (especially when using Elasticsearch) * Search for C * Check display, especially search options, options at the top of the page etc. * Apply patch * Verify changes listed above * Regression test the normal search results, all should look the same with and without patch. * If you think the scan index page is nicer now - sign off.
I fixed a problem with the dependent bug and forgot to rebase this one on top.
Created attachment 149625 [details] [review] Bug 33505: Improve styling of scan index page This patch makes some changes to the 'scan index' page. * Updates second h1 heading "Scan index:" to h2 level heading "Scan index" * Removes table from the search form * Moves search index pull down and search term into one line * Reorders options with search index first and search term after, matching the advanced search form * Removes the catalog results toolbar from the top, card hold etc. are not needed here. * Fixes doubled up id attribute: scan-index by turning one into scan-index-term. To test: * Go to advanced search in staff interface * Activate 'More options' * Check checkbox for scan indexes * Switch to author (especially when using Elasticsearch) * Search for C * Check display, especially search options, options at the top of the page etc. * Apply patch * Verify changes listed above * Regression test the normal search results, all should look the same with and without patch. * If you think the scan index page is nicer now - sign off.
Rebased!
This is an improvement, here are some additional things I noted: 1. Zebra and Elasticsearch result differences: - when using Zebra, only 20 results are displayed - it looks to me like the pagination of search results is missing, or maybe it is too difficult. - when using Elasticsearch, there is pagination at the bottom (like the normal staff interface catalog search results) 2. The results pages need the page section added (bug 31757).
It definitely looks better! There is a small typo around line 376: [% ELSE %]<option selected="selected" value="">Any word</option[% END %] should be: [% ELSE %]<option selected="selected" value="">Any word</option>[% END %]
Created attachment 149713 [details] [review] Bug 33505: Improve styling of scan index page This patch makes some changes to the 'scan index' page. * Updates second h1 heading "Scan index:" to h2 level heading "Scan index" * Removes table from the search form * Moves search index pull down and search term into one line * Reorders options with search index first and search term after, matching the advanced search form * Removes the catalog results toolbar from the top, card hold etc. are not needed here. * Fixes doubled up id attribute: scan-index by turning one into scan-index-term. To test: * Go to advanced search in staff interface * Activate 'More options' * Check checkbox for scan indexes * Switch to author (especially when using Elasticsearch) * Search for C * Check display, especially search options, options at the top of the page etc. * Apply patch * Verify changes listed above * Regression test the normal search results, all should look the same with and without patch. * If you think the scan index page is nicer now - sign off.
(In reply to Lucas Gass from comment #7) > It definitely looks better! There is a small typo around line 376: > > > [% ELSE %]<option selected="selected" value="">Any word</option[% END %] > > > should be: > > > [% ELSE %]<option selected="selected" value="">Any word</option>[% END %] Hi Lucas, thx for noticing :) I have fixed that for the third (and hopefully last time) now. I hope this one for good (had some problems with the branches building on each other and rebases :( )
Created attachment 150299 [details] [review] Bug 33505: Improve styling of scan index page This patch makes some changes to the 'scan index' page. * Updates second h1 heading "Scan index:" to h2 level heading "Scan index" * Removes table from the search form * Moves search index pull down and search term into one line * Reorders options with search index first and search term after, matching the advanced search form * Removes the catalog results toolbar from the top, card hold etc. are not needed here. * Fixes doubled up id attribute: scan-index by turning one into scan-index-term. To test: * Go to advanced search in staff interface * Activate 'More options' * Check checkbox for scan indexes * Switch to author (especially when using Elasticsearch) * Search for C * Check display, especially search options, options at the top of the page etc. * Apply patch * Verify changes listed above * Regression test the normal search results, all should look the same with and without patch. * If you think the scan index page is nicer now - sign off. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to David Nind from comment #6) > This is an improvement, here are some additional things I noted: > > 1. Zebra and Elasticsearch result differences: > - when using Zebra, only 20 results are displayed - it looks to me like > the pagination of search results is missing, or maybe it is too difficult. > - when using Elasticsearch, there is pagination at the bottom (like the > normal staff interface catalog search results) This is because the Zebra scan only returns a single page of results, not a full count - I don't know why, it seems to be a variable we pass/set somewhere, but the scan size is always limited - much deeper bug than this
(In reply to Nick Clemens from comment #11) > (In reply to David Nind from comment #6) > > This is an improvement, here are some additional things I noted: > > > > 1. Zebra and Elasticsearch result differences: > > - when using Zebra, only 20 results are displayed - it looks to me like > > the pagination of search results is missing, or maybe it is too difficult. > > - when using Elasticsearch, there is pagination at the bottom (like the > > normal staff interface catalog search results) > > This is because the Zebra scan only returns a single page of results, not a > full count - I don't know why, it seems to be a variable we pass/set > somewhere, but the scan size is always limited - much deeper bug than this Thanks Nick for explaining.
> 2. The results pages need the page section added (bug 31757). Moved to: Bug 33643 - Add page-section to 'scan index' page
Pushed to master for 23.05. Nice work everyone, thanks!
Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release
Backported to 22.05.x for upcoming 22.05.13