Description
Katrin Fischer
2022-10-30 12:31:41 UTC
Created attachment 143258 [details]
Proposed layout change
How does this look as an alternative layout?
The layout would work for me - maybe some more white background? Owen, will you have time for this one? I could try to have a go at the misaligned statistics and the item form (looking at the other places we fixed them) Created attachment 143952 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Created attachment 143956 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 143958 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion I think I tested this correctly, but have a couple of comments: 1. Could we make an exception to the add/edit item form so that the width for form labels is wider? In my view, making it narrow and then wrapping the text: - makes it harder to read/scan - makes the input field height longer - which makes the alignment for the tick boxes and RegEx links look "weird" (Batch item modification form) - particularly when there is enough space on the page, even for smaller screen widths 2. For the acquisition screens, I wasn't sure why on these that the dropdown lists have a grey background. The first tab looks great now, the third tab too, but I got some issues with the item form: https://snipboard.io/QHUtIW.jpg The longer descriptions now blow up the input fields, this was not the case in previous iterations. In general I'd love if we could give labels more space in multiple locations. We always need to add additional CSS there for German and I'd so love to get rid of it. Also the problem seems prominent now also for the English templates. Great minds think alike ;) Created attachment 143996 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion Created attachment 143997 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion I am sorry, conflicts... :( Created attachment 144022 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Created attachment 144023 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion Created attachment 144027 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Created attachment 144028 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion *** Bug 32235 has been marked as a duplicate of this bug. *** FAIL koha-tmpl/intranet-tmpl/prog/en/modules/acqui/addorderiso2709.tt OK filters OK forbidden patterns OK git manipulation FAIL js_in_body A <script> tag found inside head, must be moved to the body (see bug 17858) It's a false positive, I checked with Owen when I signed off and he also noted in the commit:
> Note that the QA tool's js_in_body error is invalid for this template
> which requires that the JS stay inside <head>.
Ah, I see: [%# As long as cataloging plugins rely on 'script' tags added inline, JS must be in the header %] <script> const template_path = "[% interface | html %]/[% theme | html %]"; </script> Katrin: You switched status. But you did not add signoff lines ? Actually, there are such large changes in a template that I cant oversee if we somewhere add string changes at this time before release.. (In reply to Marcel de Rooy from comment #21) > Actually, there are such large changes in a template that I cant oversee if > we somewhere add string changes at this time before release.. Ah, will fix. It's my usual mistake when I forget to git stash the yarn. The git so then fails a bit too silently. Created attachment 144052 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 144053 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 144193 [details] [review] Bug 32046: When adding a new records from a staged files, there are style issues This patch modifies the template for adding a record from a staged file in order to resolve some layout problems introduced by the staff redesign. This patch also corrects an problem with addorderiso2709.js: It included some template markup. The template variables are converted to a JS variable to prevent a JS error. Note that the QA tool's js_in_body error is invalid for this template which requires that the JS stay inside <head>. To test you must have at least one MARC file staged for import. - Apply the patch and go to Acquisitions. - Locate a vendor and create a basket if necessary. - Add to the basket and choose the option of adding an order from a staged file. - Click "Add orders" next to one of the staged files. - On the Add orders from MARC file page, test that everything looks correct and works correctly: - In the first tab, expanding and collapsing order details by checking the checkbox or clicking the title. - Test the "MARC" and "Card" preview buttons. - Under the Item information tab, confirm that the layout looks correct and that the form works correctly, including plugin operations like auto barcode fill. - Test that, under the Default accounting details tab, changing the fund automatically sets the fund for each title in the first tab. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 144194 [details] [review] Bug 32046: (follow-up) Accommodate the inclusion of item information If you have item information configured in the MarcItemFieldsToOrder system preference, the "Select to import" tab will display a basic item information form which needs to use the same style as other item edit forms. In order to make the CSS in addbiblio.css more generic, I've changed some styles to use a class, .item_edit_form, instead of an ID. This lets us have multiple <div class="item_edit_form"> in this template without duplicating IDs. Other instances of <div id="cataloguing_additem_newitem"> have haa the .item_edit_form class added. To test you must have a staged MARC file which includes items and you must configure the MarcItemFieldsToOrder preference to map those items in the ordering interface. As you follow the previous patch's test plan you'll find that when you click a checkbox or title to expand the information about that title you'll see "Item record X" headings following by a properly-styled form. To confirm that the changes to addbiblio.css are not too far-reaching, test the other affected pages: - Item add/edit - Batch item modification - Batch item deletion Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Nice improvement, QA scripts generally happy and all works as expected. Passing QA. Warning: This patch does introduce two new strings as far as I can tell, labels only. I'll leave it to RM's discretion whether as a major it's worth the pain or not to push now or just after 22.11.00 release. Pushed to master for 22.11. Nice work everyone, thanks! |