Bug 33505 - Improve styling of scan index page
Summary: Improve styling of scan index page
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords: Academy
Depends on: 33506
Blocks: 33643
  Show dependency treegraph
 
Reported: 2023-04-12 12:27 UTC by Katrin Fischer
Modified: 2023-12-28 20:44 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06,22.05.13


Attachments
Bug 33505: Improve styling of scan index page (10.98 KB, patch)
2023-04-12 21:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33505: Improve styling of scan index page (10.76 KB, patch)
2023-04-12 21:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33505: Improve styling of scan index page (10.64 KB, patch)
2023-04-13 20:12 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33505: Improve styling of scan index page (10.65 KB, patch)
2023-04-15 21:41 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33505: Improve styling of scan index page (10.70 KB, patch)
2023-04-27 13:33 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2023-04-12 12:27:48 UTC
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
Comment 1 Katrin Fischer 2023-04-12 21:09:56 UTC
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.
Comment 2 Katrin Fischer 2023-04-12 21:12:50 UTC
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.
Comment 3 Katrin Fischer 2023-04-13 11:38:06 UTC
I fixed a problem with the dependent bug and forgot to rebase this one on top.
Comment 4 Katrin Fischer 2023-04-13 20:12:33 UTC
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.
Comment 5 Katrin Fischer 2023-04-13 20:13:23 UTC
Rebased!
Comment 6 David Nind 2023-04-14 00:56:05 UTC
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).
Comment 7 Lucas Gass 2023-04-14 14:23:46 UTC
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 %]
Comment 8 Katrin Fischer 2023-04-15 21:41:16 UTC
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.
Comment 9 Katrin Fischer 2023-04-15 21:42:10 UTC
(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 :( )
Comment 10 Nick Clemens 2023-04-27 13:33:10 UTC
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>
Comment 11 Nick Clemens 2023-04-27 14:20:32 UTC
(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
Comment 12 David Nind 2023-04-27 20:25:29 UTC
(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.
Comment 13 Katrin Fischer 2023-05-01 13:22:42 UTC
> 2. The results pages need the page section added (bug 31757).

Moved to:
Bug 33643 - Add page-section to 'scan index' page
Comment 14 Tomás Cohen Arazi 2023-05-05 12:17:43 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 15 Martin Renvoize 2023-05-11 12:25:43 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 16 Lucas Gass 2023-05-15 22:32:21 UTC
Backported to 22.05.x for upcoming 22.05.13