Since preference OpacMainUserBlock is an HTML customization, there is an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". I think template block "koha_news_block" should add a class instead of an id. It fixes the bug and also it may be used several times in same page in the future.
Created attachment 124267 [details] [review] Since preference OpacMainUserBlock is an HTML customization, there is an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". I think template block "koha_news_block" should add a class (lowercase) instead of an id. It fixes the bug and also it may be used several times in same page in the future. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see class="opacmainuserblock" and not id="OpacMainUserBlock"
Created attachment 124318 [details] [review] Since preference OpacMainUserBlock is an HTML customization, there is an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". I think template block "koha_news_block" should add a class (lowercase) instead of an id. It fixes the bug and also it may be used several times in same page in the future. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see class="opacmainuserblock" and not id="OpacMainUserBlock" https://bugs.koha-community.org/show_bug.cgi?id=28927 Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Works as described.
Thanks, I think this change looks reasonable, though some things to fix. 1) Could you fix up the commit message, you mention that this fixes a bug but don't say which one, also mention in the commit message what the change does and use the format "Bug XXXX: Change X thing". I'm guessing you are doing this change to simplify (custom) CSS rules? 2) It seems to break the OpacHeader / opacheader CSS rule defined in the line koha/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss:#opacheader {
(In reply to Joonas Kylmälä from comment #4) > Thanks, I think this change looks reasonable, though some things to fix. > > 1) Could you fix up the commit message, you mention that this fixes a bug > but don't say which one, also mention in the commit message what the change > does and use the format "Bug XXXX: Change X thing". I'm guessing you are > doing this change to simplify (custom) CSS rules? Oups sorry i did a git mistake. > > 2) It seems to break the OpacHeader / opacheader CSS rule defined in the line > > koha/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss:#opacheader { Good point i'm on it
Created attachment 124637 [details] [review] Bug 28927: Fix id opacmainuserblock used twice in OPAC Since preference OpacMainUserBlock is an HTML customization, there is an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". I think template block "koha_news_block" should add a class (lowercase) instead of an id. It fixes the bug and also it may be used several times in same page in the future. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see class="opacmainuserblock" and not id="OpacMainUserBlock" https://bugs.koha-community.org/show_bug.cgi?id=28927 Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
(In reply to Joonas Kylmälä from comment #4) > > 2) It seems to break the OpacHeader / opacheader CSS rule defined in the line > > koha/koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss:#opacheader { Actually this rule does nothing, it defines same background color as "body" : #opacheader { background-color: #fcf9fc; } I propose de remove it.
Created attachment 124638 [details] [review] Bug 28927: (follow-up) remove rule on id=opacheader in opac CSS Actually this rule does nothing, it defines same background color as "body" : #fcf9fc Test plan : 1) Without patch look at OPAC header 2) With patch OPAC header should have same background color
Created attachment 124793 [details] [review] Bug 28927: Fix id opacmainuserblock used twice in OPAC Since preference OpacMainUserBlock is an HTML customization, there is an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". I think template block "koha_news_block" should add a class (lowercase) instead of an id. It fixes the bug and also it may be used several times in same page in the future. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see class="opacmainuserblock" and not id="OpacMainUserBlock" https://bugs.koha-community.org/show_bug.cgi?id=28927 Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Rebased-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Created attachment 124794 [details] [review] Bug 28927: (follow-up) remove rule on id=opacheader in opac CSS Actually this rule does nothing, it defines same background color as "body" : #fcf9fc Test plan : 1) Without patch look at OPAC header 2) With patch OPAC header should have same background color Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Passing QA as Sally already signed this off.
Current behaviour is definitely wrong, but I am wondering why we don't keep the id with the same capitalization (not lowered).
Asked Owen on IRC: oleonard> Hm, no... There should be a single OpacMainUserBlock, not two. The class would be correct if it were repeated for each additional content entry, but it's not. Joubu> yes, that's my point oleonard> I agree with the change removing the divs from opac-main.tt and opac-registration-confirmation.tt, those are cleanups I should have done earlier. oleonard> However I think the id in html_helpers.inc should stay.
Created attachment 125125 [details] [review] Bug 28927: Fix id opacmainuserblock used twice in OPAC Since preference OpacMainUserBlock is now an HTML customization, there is actually an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". Same for OPACNavRight in opac-registration-confirmation.tt This patch removed id from page to keep the one comming from HTML customization include. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see id="OpacMainUserBlock" once and not id="opacmainuserblock" 5) Same for id="opacnavright" in opac-registration-confirmation.tt
Created attachment 125126 [details] [review] Bug 28927: (follow-up) remove rule on id=opacheader in opac CSS Actually this rule does nothing, it defines same background color as "body" : #fcf9fc Test plan : 1) Without patch look at OPAC header 2) With patch OPAC header should have same background color Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
(In reply to Jonathan Druart from comment #13) > Asked Owen on IRC: > > oleonard> Hm, no... There should be a single OpacMainUserBlock, not two. The > class would be correct if it were repeated for each additional content > entry, but it's not. > Joubu> yes, that's my point > oleonard> I agree with the change removing the divs from opac-main.tt and > opac-registration-confirmation.tt, those are cleanups I should have done > earlier. > oleonard> However I think the id in html_helpers.inc should stay. Ok I'm in. Main patch needs a new signoff. I've kept followup patch if its OK.
Created attachment 125137 [details] [review] Bug 28927: Fix id opacmainuserblock used twice in OPAC Since preference OpacMainUserBlock is now an HTML customization, there is actually an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". Same for OPACNavRight in opac-registration-confirmation.tt This patch removed id from page to keep the one comming from HTML customization include. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see id="OpacMainUserBlock" once and not id="opacmainuserblock" 5) Same for id="opacnavright" in opac-registration-confirmation.tt https://bugs.koha-community.org/show_bug.cgi?id=28927 Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 125138 [details] [review] Bug 28927: (follow-up) remove rule on id=opacheader in opac CSS Actually this rule does nothing, it defines same background color as "body" : #fcf9fc Test plan : 1) Without patch look at OPAC header 2) With patch OPAC header should have same background color Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 125332 [details] [review] Bug 28927: Fix id opacmainuserblock used twice in OPAC Since preference OpacMainUserBlock is now an HTML customization, there is actually an HTML tags with id="opacmainuserblock" and one with id="OpacMainUserBlock". Same for OPACNavRight in opac-registration-confirmation.tt This patch removed id from page to keep the one comming from HTML customization include. Test plan : 1) Create an HTML customization for OpacMainUserBlock with some text 2) Go to OPAC main page 3) Check HTML code of the page 4) Check you see id="OpacMainUserBlock" once and not id="opacmainuserblock" 5) Same for id="opacnavright" in opac-registration-confirmation.tt https://bugs.koha-community.org/show_bug.cgi?id=28927 Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 125333 [details] [review] Bug 28927: (follow-up) remove rule on id=opacheader in opac CSS Actually this rule does nothing, it defines same background color as "body" : #fcf9fc Test plan : 1) Without patch look at OPAC header 2) With patch OPAC header should have same background color Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Pushed to master for 21.11, thanks to everybody involved!
Pushed to 21.05.x for 21.05.05
Pushed to 20.11.x for 20.11.11
Not backported to oldoldstable (20.05.x). Feel free to ask if it's needed.