Bug 18820

Summary: The different parts in the main don't automatically adjust with the available space
Product: Koha Reporter: Philippe Audet-Fortin <philippe.audet-fortin>
Component: TemplatesAssignee: Roch D'Amour <roch.damour>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: dominic.pichette, jonathan.druart, katrin.fischer, m.de.rooy, nick, philippe.audet-fortin, roch.damour
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
[SIGNED-OFF] Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
before the patch was applied
with the patch, opacuserlogin=false and opacnac=false
with the patch, opacuserlogin=true and opacnav=false
with the patch, opacuserlogin=true and opacnav=true
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin

Description Philippe Audet-Fortin 2017-06-19 13:11:11 UTC
If opacuserlogin is disabled and you are on the opac homepage, you can see that there is an empty space on the right where the login form is supposed to be. It would be better if the main content took all available space.
Comment 1 Philippe Audet-Fortin 2017-06-19 13:39:41 UTC
Created attachment 64424 [details] [review]
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.
Comment 2 Philippe Audet-Fortin 2017-06-19 13:56:04 UTC
The patch check if opacuserlogin is enable and if OpacNav has content. Depending on what is true or false the main content is wrap in a div spanXX.

Test plan:
1)In the intranet, disable opacuserlogin
2)Go on the Opac homepage. There will be an empty space on the right of the main block where the login form was.
3)Apply the patch.
4)There should be no more empty space.
5)Remove all content from the OpacNav and the main content should take all available space.
6)Enable opacuserlogin and leave OpacNav empty and the main content should take all available space.
Comment 3 Owen Leonard 2017-06-20 12:24:58 UTC
Created attachment 64447 [details] [review]
[SIGNED-OFF] Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Nick Clemens 2017-09-01 10:01:14 UTC
Created attachment 66698 [details] [review]
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Marcel de Rooy 2017-09-01 10:08:27 UTC
(In reply to Nick Clemens from comment #4)
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Please read QA contact field !
Comment 6 Marcel de Rooy 2017-09-01 10:12:05 UTC
Yes, I was testing it too and would have passed qa on it.
But I could not manange somehow to make this patch show a difference. Followed the test plan and emptied Nav NavBottom MainUserBlock. Refreshing etc. no difference?
Comment 7 Jonathan Druart 2017-09-01 14:51:00 UTC
(In reply to Marcel de Rooy from comment #6)
> Yes, I was testing it too and would have passed qa on it.
> But I could not manange somehow to make this patch show a difference.
> Followed the test plan and emptied Nav NavBottom MainUserBlock. Refreshing
> etc. no difference?

Same, I cannot see any difference.
Could you explain a bit more?

Philippe, the author's name of the patch is Paudet, is it what you want? It will appear as it in the release notes.
Comment 8 Dominic Pichette 2017-09-12 18:21:39 UTC
Created attachment 67101 [details] [review]
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 9 Dominic Pichette 2017-09-12 18:25:18 UTC
(In reply to Dominic Pichette from comment #8)
> Created attachment 67101 [details] [review] [review]
> Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav
> has content.
> 
> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
> 
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
> Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Note: You have to generate the page in English since other languages are excluded from git and, therefore, not changed by the patch. This might have been the problem for Jonathan and Marcel?
Comment 10 Dominic Pichette 2017-09-12 19:08:39 UTC
Created attachment 67103 [details]
before the patch was applied
Comment 11 Dominic Pichette 2017-09-12 19:09:20 UTC
Created attachment 67104 [details]
with the patch, opacuserlogin=false and opacnac=false
Comment 12 Dominic Pichette 2017-09-12 19:10:31 UTC
Created attachment 67105 [details]
with the patch, opacuserlogin=true and opacnav=false
Comment 13 Dominic Pichette 2017-09-12 19:11:03 UTC
Created attachment 67106 [details]
with the patch, opacuserlogin=true and opacnav=true
Comment 14 Dominic Pichette 2017-09-12 19:21:55 UTC
Just to make sure I'm clear. I attached some screenshots to show the difference with the result before the patch.

- "beforepatch.png" shows the result you get when opacuserlogin and opacnav are set to false.
- "opacuserlogin=f_opacnav=f.png" shows the result with the patch and opacuserlogin and opacnav set to false
- "opacuserlogin=t_opacnav=f.png" shows the result with the patch and opacuserlogin set to true and opacnav set to false

So what I meant in my last comment was that you always get the "beforepatch.png" result when you're not reading the page in english but the patch works properly otherwise as can be seen in the two other pictures.
Comment 15 Kyle M Hall 2017-10-27 14:47:57 UTC
Created attachment 68736 [details] [review]
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Jonathan Druart 2017-10-27 18:22:48 UTC
I need another confirmation.

Before the patch: https://screenshots.firefox.com/i220lvWCoUM1Mlf8/catalogue.kohadev.org
After the patch: https://screenshots.firefox.com/PBXfxyLVaP1tHIXm/catalogue.kohadev.org

Could you confirm it is what we want?
Comment 17 Owen Leonard 2017-10-27 18:49:29 UTC
I'm not sure if this is something I missed or if this is a new problem, but with opacuserlogin off OpacNavRight is misplaced.
Comment 18 Roch D'Amour 2018-02-22 21:44:28 UTC
Created attachment 72098 [details] [review]
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin

Test plans  :
- "beforepatch.png" shows the result you get when OpacNavRight and opacnav are set to false.
- "opacuserlogin=f_opacnav=f.png" shows the result with the patch and OpacNavRight and opacnav set to false
- "opacuserlogin=t_opacnav=f.png" shows the result with the patch and OpacNavRight set to true and opacnav set to false
Comment 19 Dilan Johnpullé 2018-02-28 04:42:48 UTC
Created attachment 72284 [details] [review]
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin

Test plans  :
- "beforepatch.png" shows the result you get when OpacNavRight and opacnav are set to false.
- "opacuserlogin=f_opacnav=f.png" shows the result with the patch and OpacNavRight and opacnav set to false
- "opacuserlogin=t_opacnav=f.png" shows the result with the patch and OpacNavRight set to true and opacnav set to false

Applied patch, can confirm it functions as expected.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 20 Katrin Fischer 2018-03-12 16:11:46 UTC
I tested in Chrome: Turn opacuserlogin off, put text in OpacMainUserBlock and opacnavright. OpacNavRight ends up on the left side inside of OpacMainUserBlock. Something is not right there.

To test:
OpacNavRight: Just put some text
OpacMainUserBlock: <div style="background-color:green; width:100%> </div>
Comment 21 Katrin Fischer 2018-03-13 16:15:27 UTC
Resetting - I think the error was on my side!
Comment 22 Katrin Fischer 2018-03-31 07:50:22 UTC
Created attachment 73508 [details] [review]
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2018-03-31 07:55:59 UTC
Created attachment 73509 [details] [review]
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin

Test plans  :
- "beforepatch.png" shows the result you get when OpacNavRight and opacnav are set to false.
- "opacuserlogin=f_opacnav=f.png" shows the result with the patch and OpacNavRight and opacnav set to false
- "opacuserlogin=t_opacnav=f.png" shows the result with the patch and OpacNavRight set to true and opacnav set to false

Applied patch, can confirm it functions as expected.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Jonathan Druart 2018-04-02 21:14:15 UTC
Patches pushed to master.

Congratulations Roch and Philippe for your first patches in!
Comment 25 Nicolas Legrand 2018-04-03 08:54:32 UTC
Created attachment 73547 [details] [review]
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin

Test plans  :
- "beforepatch.png" shows the result you get when OpacNavRight and opacnav are set to false.
- "opacuserlogin=f_opacnav=f.png" shows the result with the patch and OpacNavRight and opacnav set to false
- "opacuserlogin=t_opacnav=f.png" shows the result with the patch and OpacNavRight set to true and opacnav set to false

Applied patch, can confirm it functions as expected.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Nick Clemens 2018-05-14 14:36:25 UTC
Seems more like enhancement, not backporting to 17.11.x