|
Description
Marc Véron
2017-05-04 15:29:51 UTC
Created attachment 63127 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. First draft, not ready to sign off. Created attachment 63130 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. Amended to apply on top of bug 18465 Draft, not ready to sign off. Created attachment 63162 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. Amended to apply on top of bug 18465 Amended to show info about units Draft, not ready to sign off. Created attachment 63217 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. To test: - Apply on top of patch 18465 - Go to Home > Tools > Patron card creator - Edit or create a layout - Turn on new choice 'Guide grid' in section 'General settings' - Leave 'Units' unchanged - Crate a PDF using 'Card batches' - Notice that card is printed with a layout grid that reflects selected unit with each 5th and 10th line in different color unit description displayed bottom left, card dimensions dispayed bottom right in small print inside the layout grid - Print PDF. Set printer settings in Adobe Reader or other PDF printing software to 'Actual size' to prevent scaling to printer's printable region - Mesure out printed PDF and verify that grid corresponds to selecte unit. - Go back to layout definition and choose an other unit, repeat steps to verify that grid respects - Go back to layout definition, turn grid off, create PDF, vrify that grid does not display in PDF Note for testers / QAers: Position of card elements (text, image...) do not respect the unit, this will be fixed in Bug 18550 Created attachment 63218 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. To test: - Apply on top of patch 18465 - Go to Home > Tools > Patron card creator - Edit or create a layout - Turn on new choice 'Guide grid' in section 'General settings' - Leave 'Units' unchanged - Crate a PDF using 'Card batches' - Notice that card is printed with a layout grid that reflects selected unit with each 5th and 10th line in different color, unit description displayed bottom left, card dimensions displayed bottom right in small print inside the layout grid - Print PDF. Set printer settings in Adobe Reader or other PDF printing software to 'Actual size' to prevent scaling to printer's printable region - Mesure out printed PDF and verify that grid corresponds to selecte unit. - Go back to layout definition and choose an other unit, repeat steps to verify that grid respects selected unit. - Go back to layout definition, turn grid off, create PDF, verify that grid does not display in PDF Note for testers / QAers: Position of card elements (text, image...) do not respect the unit, this will be fixed in Bug 18550 Created attachment 63263 [details]
Unit description and card dimension on patron card batch
Hi Marc
This patch works well but I have one question:
You say in your test plan that the card batches should have the unit description displayed in the bottom left (which I have got) but the card dimensions should be in the bottom right however I have found mine is in the top right (see screenshot) is this what you intended?
(In reply to Alex Buckley from comment #6) > Created attachment 63263 [details] > Unit description and card dimension on patron card batch > > Hi Marc > > This patch works well but I have one question: > > You say in your test plan that the card batches should have the unit > description displayed in the bottom left (which I have got) but the card > dimensions should be in the bottom right however I have found mine is in the > top right (see screenshot) is this what you intended? Hi Alex, Yes indeed, I misaked with my description. It should appear top right (as you describe) to indicate that the coordinate system is from bottom left = 0/0 to top right. If you sign off, can you please change the test plan in the patch attached? Thanks! Marc Created attachment 63280 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. To test: - Apply on top of patch 18465 - Go to Home > Tools > Patron card creator - Edit or create a layout - Turn on new choice 'Guide grid' in section 'General settings' - Leave 'Units' unchanged - Crate a PDF using 'Card batches' - Notice that card is printed with a layout grid that reflects selected unit with each 5th and 10th line in different color, unit description displayed bottom left, card dimensions displayed top right in small print inside the layout grid - Print PDF. Set printer settings in Adobe Reader or other PDF printing software to 'Actual size' to prevent scaling to printer's printable region - Mesure out printed PDF and verify that grid corresponds to selecte unit. - Go back to layout definition and choose an other unit, repeat steps to verify that grid respects selected unit. - Go back to layout definition, turn grid off, create PDF, verify that grid does not display in PDF Note for testers / QAers: Position of card elements (text, image...) do not respect the unit, this will be fixed in Bug 18550 Followed test plan and it worked as intended Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Still applies on current master Still applies on current master. - $patron_card->draw_guide_box($pdf) if $print_layout_xml->{'guide_box'};
- $patron_card->draw_barcode($pdf) if $print_layout_xml->{'barcode'};
+ $patron_card->draw_guide_box($pdf) if $layout_xml->{'guide_box'};
+ $patron_card->draw_guide_grid($pdf) if $layout_xml->{'guide_grid'};
+ $patron_card->draw_barcode($pdf) if $layout_xml->{'barcode'};
Marc,
Please explain this change.
Formerly, as I understand, it flipped between layout_xml and layout_back_xml. Now you always set it to layout_xml. Do you change behavior for draw_guide_box and draw_barcode ?
Status change to reflect need for feedback only.
Forgot to mention that qa tools reports a fail on pod somewhere too (In reply to Marcel de Rooy from comment #11) > - $patron_card->draw_guide_box($pdf) if > $print_layout_xml->{'guide_box'}; > - $patron_card->draw_barcode($pdf) if $print_layout_xml->{'barcode'}; > + $patron_card->draw_guide_box($pdf) if $layout_xml->{'guide_box'}; > + $patron_card->draw_guide_grid($pdf) if $layout_xml->{'guide_grid'}; > + $patron_card->draw_barcode($pdf) if $layout_xml->{'barcode'}; > > Marc, > Please explain this change. > Formerly, as I understand, it flipped between layout_xml and > layout_back_xml. Now you always set it to layout_xml. Do you change behavior > for draw_guide_box and draw_barcode ? > > Status change to reflect need for feedback only. Marcel, Thanks for QAing and pointing this out. Yes, it should be $print_layout_xml, not $layout_xml. Follow-up patch will follow. Marc (In reply to Marcel de Rooy from comment #12) > Forgot to mention that qa tools reports a fail on pod somewhere too I can not reproduce with patch applied: Processing files before patches |========================>| 4 / 4 (100.00%) Processing files after patches |========================>| 4 / 4 (100.00%) OK C4/Patroncards/Patroncard.pm OK critic OK forbidden patterns OK git manipulation OK pod OK spelling OK valid OK koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-layout.tt OK forbidden patterns OK git manipulation OK spelling OK tt_valid OK valid_template OK patroncards/create-pdf.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling OK valid OK patroncards/edit-layout.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling OK valid (In reply to Marc Véron from comment #14) > (In reply to Marcel de Rooy from comment #12) > > Forgot to mention that qa tools reports a fail on pod somewhere too > > I can not reproduce with patch applied: Thx for checking. If you submit your follow-up, I will check again.. Should be the pod for new sub draw_guide_grid btw.. (In reply to Marcel de Rooy from comment #16) > Should be the pod for new sub draw_guide_grid btw.. I understand that it will be hard to provide unit tests for this sub btw, as I should officially ask you for too. Maybe some advanced mocking session? (In reply to Marcel de Rooy from comment #17) > (In reply to Marcel de Rooy from comment #16) > > Should be the pod for new sub draw_guide_grid btw.. > > I understand that it will be hard to provide unit tests for this sub btw, as > I should officially ask you for too. Maybe some advanced mocking session? Sorry, that is out of my scope. If you insist I will have to abandon this bug. Created attachment 66873 [details] [review] Bug 18541: (QA follow-up) for comments #11 and #13 This patch changes variable names as mentioned in comments #11 and #13 (In reply to Marcel de Rooy from comment #16) > Should be the pod for new sub draw_guide_grid btw.. There aren't any PODs in Patroncards.pm, I think that is for historical reasons. Does it make sense to add a pod just for one sub, or is it better to create a new bug "Add PODs to Patroncards.pm"? Created attachment 66876 [details] [review] Bug 18541 - Patron card creator: Add a grid to support layout design Add a layout grid to patron card creator to figure out the positions of text fields, barcode and images. To test: - Apply on top of patch 18465 - Go to Home > Tools > Patron card creator - Edit or create a layout - Turn on new choice 'Guide grid' in section 'General settings' - Leave 'Units' unchanged - Crate a PDF using 'Card batches' - Notice that card is printed with a layout grid that reflects selected unit with each 5th and 10th line in different color, unit description displayed bottom left, card dimensions displayed top right in small print inside the layout grid - Print PDF. Set printer settings in Adobe Reader or other PDF printing software to 'Actual size' to prevent scaling to printer's printable region - Mesure out printed PDF and verify that grid corresponds to selecte unit. - Go back to layout definition and choose an other unit, repeat steps to verify that grid respects selected unit. - Go back to layout definition, turn grid off, create PDF, verify that grid does not display in PDF Note for testers / QAers: Position of card elements (text, image...) do not respect the unit, this will be fixed in Bug 18550 Followed test plan and it worked as intended Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66877 [details] [review] Bug 18541: (QA follow-up) for comments #11 and #13 This patch changes variable names as mentioned in comments #11 and #13 Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66878 [details] [review] Bug 18541: [QA Follow-up] Add POD to Patroncard.pm Just adding the POD framework to make qa tools happy. The authors are encouraged to complete this information. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Marc Véron from comment #20) > (In reply to Marcel de Rooy from comment #16) > > Should be the pod for new sub draw_guide_grid btw.. > > There aren't any PODs in Patroncards.pm, I think that is for historical > reasons. > > Does it make sense to add a pod just for one sub, or is it better to create > a new bug "Add PODs to Patroncards.pm"? I added a follow-up. Please add another follow-up to complete the pod for the new sub. Thanks. (In reply to Marc Véron from comment #18) > Sorry, that is out of my scope. If you insist I will have to abandon this > bug. I will not insist. I recommend to push it, but the RM has final word. (In reply to Marc Véron from comment #14) > I can not reproduce with patch applied: You probably are not using the latest version of qa tools. Created attachment 66887 [details] [review] Bug 18541: (QA follow-up) for comment #24 (POD) Add description to POD for draw_guide_grid (In reply to Marcel de Rooy from comment #23) > Created attachment 66878 [details] [review] [review] > Bug 18541: [QA Follow-up] Add POD to Patroncard.pm > > Just adding the POD framework to make qa tools happy. The authors > are encouraged to complete this information. > > Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> I filed Bug 19264 for completing all PODs in Patroncard.pm Created attachment 66910 [details] [review] Bug 18541: (QA follow-up) for comment #24 (POD) Add description to POD for draw_guide_grid Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended: Moved the description from draw_guide_box to .._grid. Pushed to master for 17.11, thanks to everybody involved! Enhancement not pushed to 17.05.x |