Bug 18820 - The different parts in the main don't automatically adjust with the available space
Summary: The different parts in the main don't automatically adjust with the available...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Roch D'Amour
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-19 13:11 UTC by Philippe Audet-Fortin
Modified: 2018-05-14 14:36 UTC (History)
7 users (show)

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


Attachments
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.30 KB, patch)
2017-06-19 13:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.37 KB, patch)
2017-06-20 12:24 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.41 KB, patch)
2017-09-01 10:01 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.46 KB, patch)
2017-09-12 18:21 UTC, Dominic Pichette
Details | Diff | Splinter Review
before the patch was applied (287.13 KB, image/png)
2017-09-12 19:08 UTC, Dominic Pichette
Details
with the patch, opacuserlogin=false and opacnac=false (303.23 KB, image/png)
2017-09-12 19:09 UTC, Dominic Pichette
Details
with the patch, opacuserlogin=true and opacnav=false (298.62 KB, image/png)
2017-09-12 19:10 UTC, Dominic Pichette
Details
with the patch, opacuserlogin=true and opacnav=true (277.65 KB, image/png)
2017-09-12 19:11 UTC, Dominic Pichette
Details
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.52 KB, patch)
2017-10-27 14:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin (1.47 KB, patch)
2018-02-22 21:44 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin (1.66 KB, patch)
2018-02-28 04:42 UTC, Dilan Johnpullé
Details | Diff | Splinter Review
Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. (1.58 KB, patch)
2018-03-31 07:50 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin (1.73 KB, patch)
2018-03-31 07:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 18820: Main block size now adjust to OpacNavRight like it would for opacuserlogin (1.79 KB, patch)
2018-04-03 08:54 UTC, Nicolas Legrand
Details | Diff | Splinter Review

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