Bug 21089

Summary:	Overlapping elements in ordering information on acqui/supplier.pl
Product:	Koha	Reporter:	David Cook <dcook>
Component:	Acquisitions	Assignee:	Owen Leonard <oleonard>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	jesse, jose-mario.monteiro-santos, martin.renvoize, nick
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Small patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl

Description David Cook 2018-07-19 07:53:14 UTC

I haven’t checked master yet, but in the 17.xx versions, I’m noticing it is really difficult to click the dropdown for “List prices are:” in “Ordering information” on /cgi-bin/koha/acqui/supplier.pl.

When I look using the F12 tools in Chrome, I see that later “ol” elements are overlapping earlier “li” elements. So the “ol” for the tax options is sitting on top of the “li” for “List prices are:”. Only a tiny part of the “li” is clickable underneath that “ol”. 

It looks like the floats in the “li” element are causing the sizing of the “ol” to be no good. See https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Block_formatting_context. 

There are a number of strategies for fixing this but I’m not sure what the best one would be here.

Comment 1 David Cook 2019-01-07 06:05:14 UTC

Still an issue in 18.11.x, although it doesn't look like floats. Rather than the position for a few "ol" elements with the radio class are relative and that's causing the problem.

Comment 2 Owen Leonard 2019-01-07 14:05:05 UTC

Created attachment 83685 [details] [review]
Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl

This patch changes the HTML structure of the vendor edit page so that
the markup is a little simpler. CSS is modified to match. This structure
should serve as a model for other similar form structures:

 <fieldset class="rows">
     <ol>
         <li>
             <label>General label:</label>
             <label class="radio">
                 Specific label 1 <input type="radio" />
             </label>
             <label class="radio">
                 Specific label 2 <input type="radio" />
             </label>
         </li>
     </ol>
 </fieldset>

To test, apply the patch and regenerate the staff client CSS.

 - Go to Acquisitions -> Vendor search -> Vendor -> Edit vendor
 - In the "Ordering information" section, confirm that the position of
   radio buttons looks correct.
 - Confirm that radio button labels work correctly.
 - Confirm that the adjacent dropdown menus work well.

Comment 3 José-Mario Monteiro-Santos 2019-01-11 19:28:23 UTC

The radio buttons look wonky to me. Some labels are to the button's left, while others are on top of it. https://imgur.com/eAkhNYo

The dropdowns work fine.

Comment 4 Owen Leonard 2019-01-14 13:58:27 UTC

Thanks for testing José-Mario. Did you regenerate the staff client CSS before testing? 

https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client

Comment 5 José-Mario Monteiro-Santos 2019-01-14 18:08:09 UTC

I did not, that is surely why it doesn't look right.

Comment 6 David Cook 2019-01-23 03:48:53 UTC

I'm trying this now... 

Currently updating https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client to work with koha-testing-docker...

Comment 7 David Cook 2019-01-23 03:53:22 UTC

Having issues with koha-testing-docker running in Docker Desktop on Windows...

Although that reminds me of a little workaround I did so that I wouldn't have to deal with Windows...

Comment 8 David Cook 2019-01-23 04:59:44 UTC

Moved my Koha git repo into an actual Docker volume, so it exists on the Linux VM and not on my Windows host and shared as a bind mount. That removes the OS issues I seemed to be experiencing. 

I'd already increased the default disk space in the Hyper-V VM, and now I increased the CPU and RAM, and now git is working much much better in koha-testing-docker on Windows. 

Now to the actual testing...

Comment 9 David Cook 2019-01-23 05:12:34 UTC

Created attachment 84315 [details] [review]
Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl

This patch changes the HTML structure of the vendor edit page so that
the markup is a little simpler. CSS is modified to match. This structure
should serve as a model for other similar form structures:

 <fieldset class="rows">
     <ol>
         <li>
             <label>General label:</label>
             <label class="radio">
                 Specific label 1 <input type="radio" />
             </label>
             <label class="radio">
                 Specific label 2 <input type="radio" />
             </label>
         </li>
     </ol>
 </fieldset>

To test, apply the patch and regenerate the staff client CSS.

 - Go to Acquisitions -> Vendor search -> Vendor -> Edit vendor
 - In the "Ordering information" section, confirm that the position of
   radio buttons looks correct.
 - Confirm that radio button labels work correctly.
 - Confirm that the adjacent dropdown menus work well.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Comment 10 David Cook 2019-01-23 05:13:01 UTC

Thanks for this, Owen! It looks much much better with your changes.

Comment 11 David Cook 2019-01-23 05:14:00 UTC

I take it that the RM is responsible for generating the koha-tmpl/intranet-tmpl/prog/css/staff-global.css later?

Comment 12 Owen Leonard 2019-01-23 12:56:48 UTC

(In reply to David Cook from comment #11)
> I take it that the RM is responsible for generating the
> koha-tmpl/intranet-tmpl/prog/css/staff-global.css later?

That is correct.

Comment 13 Katrin Fischer 2019-01-27 13:45:54 UTC

Created attachment 84465 [details] [review]
Bug 21089: Overlapping elements in ordering information on acqui/supplier.pl

This patch changes the HTML structure of the vendor edit page so that
the markup is a little simpler. CSS is modified to match. This structure
should serve as a model for other similar form structures:

 <fieldset class="rows">
     <ol>
         <li>
             <label>General label:</label>
             <label class="radio">
                 Specific label 1 <input type="radio" />
             </label>
             <label class="radio">
                 Specific label 2 <input type="radio" />
             </label>
         </li>
     </ol>
 </fieldset>

To test, apply the patch and regenerate the staff client CSS.

 - Go to Acquisitions -> Vendor search -> Vendor -> Edit vendor
 - In the "Ordering information" section, confirm that the position of
   radio buttons looks correct.
 - Confirm that radio button labels work correctly.
 - Confirm that the adjacent dropdown menus work well.

Signed-off-by: David Cook <dcook@prosentient.com.au>

Comment 14 Nick Clemens (kidclamp) 2019-01-28 14:12:57 UTC

Awesome work all!

Pushed to master for 19.05

Comment 15 Martin Renvoize (ashimema) 2019-02-01 12:05:14 UTC

Pushed to 18.11.x for 18.11.03

Comment 16 Jesse Maseto 2019-02-06 17:27:36 UTC

Merge conflict with 18.05.x. Need rebase patch to push to 18.05.x