Bug 17487 - Improper placement of select/clear all in Z39.50/SRU search dialog
Summary: Improper placement of select/clear all in Z39.50/SRU search dialog
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Z39.50 / SRU / OpenSearch Servers (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Radek Šiman (R-Bit Technology, s.r.o.)
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-23 19:38 UTC by Radek Šiman (R-Bit Technology, s.r.o.)
Modified: 2017-06-14 22:18 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Links to "select/clear all" moved below the header tag (4.62 KB, patch)
2016-10-23 20:22 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Styling moved from style attribute into staff-global.css (4.76 KB, patch)
2016-10-24 06:36 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17487: Links to "select/clear all" moved below the header tag (4.70 KB, patch)
2016-10-24 06:56 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17487: Styling moved from style attribute into staff-global.css (4.84 KB, patch)
2016-10-24 06:56 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17487: Links to "select/clear all" moved below the header tag (4.75 KB, patch)
2016-10-25 08:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17487: Styling moved from style attribute into staff-global.css (4.89 KB, patch)
2016-10-25 08:33 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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