Bug 17487

Summary: Improper placement of select/clear all in Z39.50/SRU search dialog
Product: Koha Reporter: Radek Šiman (R-Bit Technology, s.r.o.) <rbit>
Component: Z39.50 / SRU / OpenSearch ServersAssignee: Radek Šiman (R-Bit Technology, s.r.o.) <rbit>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: black23, josef.moravec, julian.maurice, katrin.fischer, kyle, m.de.rooy, mtj
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Links to "select/clear all" moved below the header tag
Styling moved from style attribute into staff-global.css
[SIGNED-OFF] Bug 17487: Links to "select/clear all" moved below the header tag
[SIGNED-OFF] Bug 17487: Styling moved from style attribute into staff-global.css
Bug 17487: Links to "select/clear all" moved below the header tag
Bug 17487: Styling moved from style attribute into staff-global.css

Description Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-23 19:38:12 UTC
Links allowing to select/clear all Z39.50 servers have icons next to the text. These links are improperly placed in the case the localized caption is a bit longer than English version, see picture:
http://prntscr.com/cy0bey

In addition, these links are placed in an <h2> header, but it's not semantically correct.
Comment 1 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-23 20:22:30 UTC
Created attachment 56735 [details] [review]
Links to "select/clear all" moved below the header tag

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
Comment 2 Josef Moravec 2016-10-24 05:46:02 UTC
Radek,

we probably can omit the whole style attribute in span, as the span is no more in h2 tag, the style attribute isn't necessary. 

What do you think?
Comment 3 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 06:13:20 UTC
I've tried to remove it, but it looks better with the left padding. On the other hand, display:inline is useless, you're right. So? Should we remove the outer span at all, or just display:inline?
Comment 4 Josef Moravec 2016-10-24 06:22:04 UTC
(In reply to Radek Šiman (R-Bit Technology, s.r.o.) from comment #3)
> I've tried to remove it, but it looks better with the left padding. On the
> other hand, display:inline is useless, you're right. So? Should we remove
> the outer span at all, or just display:inline?

What looks better is on everyone opinion, so I'm OK with removing only display:inline

thanks
Comment 5 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 06:36:43 UTC
Created attachment 56736 [details] [review]
Styling moved from style attribute into staff-global.css

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
4) [Optionally] Set some style in IntranetUserCSS for class z3950checks
Comment 6 Josef Moravec 2016-10-24 06:56:07 UTC
Created attachment 56737 [details] [review]
[SIGNED-OFF] Bug 17487: Links to "select/clear all" moved below the header tag

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Josef Moravec 2016-10-24 06:56:15 UTC
Created attachment 56738 [details] [review]
[SIGNED-OFF] Bug 17487: Styling moved from style attribute into staff-global.css

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
4) [Optionally] Set some style in IntranetUserCSS for class z3950checks

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Jonathan Druart 2016-10-25 08:32:43 UTC
Created attachment 56834 [details] [review]
Bug 17487: Links to "select/clear all" moved below the header tag

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Jonathan Druart 2016-10-25 08:33:28 UTC
Created attachment 56835 [details] [review]
Bug 17487: Styling moved from style attribute into staff-global.css

Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
4) [Optionally] Set some style in IntranetUserCSS for class z3950checks

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 10 Marcel de Rooy 2017-01-19 11:22:06 UTC
This applies to 16.05, but also to 16.11 or master.
Version is set to 16.05. What should be done?
Comment 11 Marcel de Rooy 2017-01-19 11:24:20 UTC
Kyle: should be for master too, as confirmed by Jonathan.
Comment 12 Kyle M Hall 2017-01-20 14:11:24 UTC
Pushed to master for 17.05, thanks Radek!
Comment 13 Katrin Fischer 2017-01-22 20:28:33 UTC
Contains strings - holding this back for possible inclusion in 16.11.3.
Comment 14 Katrin Fischer 2017-02-07 07:02:06 UTC
This patch has been pushed to 16.11.x and will be in 16.11.04.
Comment 15 Julian Maurice 2017-02-09 09:06:21 UTC
Pushed to 3.22.x for 3.22.17
Comment 16 Mason James 2017-02-22 23:06:18 UTC
Pushed to 16.05.x, for 16.05.10 release