Description
Philippe Audet-Fortin
2017-06-19 13:11:11 UTC
Created attachment 64424 [details] [review] Bug 18820: Adding lines that check if opacuserlogin is enable and if OpacNav has content. 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. 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> 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> (In reply to Nick Clemens from comment #4) > Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Please read QA contact field ! 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? (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. 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> (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? Created attachment 67103 [details]
before the patch was applied
Created attachment 67104 [details]
with the patch, opacuserlogin=false and opacnac=false
Created attachment 67105 [details]
with the patch, opacuserlogin=true and opacnav=false
Created attachment 67106 [details]
with the patch, opacuserlogin=true and opacnav=true
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. 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> 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? I'm not sure if this is something I missed or if this is a new problem, but with opacuserlogin off OpacNavRight is misplaced. 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 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> 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> Resetting - I think the error was on my side! 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> 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> Patches pushed to master. Congratulations Roch and Philippe for your first patches in! 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> Seems more like enhancement, not backporting to 17.11.x |