Description
Owen Leonard
2011-10-24 12:53:03 UTC
Created attachment 6029 [details] [review] Proposed fix Created attachment 6031 [details]
pay.pl revised interface
Created attachment 6032 [details]
After clicking pay button on individual fine
Created attachment 6033 [details]
After clicking write off button on individual fine
Created attachment 6034 [details]
After clicking pay amount button
Created attachment 6035 [details]
After clicking pay selected button
Created attachment 6036 [details]
After clicking write off all button
Created attachment 6820 [details] [review] Signed off patch Patch looks good, signing off. I noticed some inconsistency in the tab titles in the fines tab for a patron. Follow up patch will follow to fix this trivial issue. Created attachment 6821 [details] [review] Changed casing on tab titles for fines page - bug 7080 Created attachment 6824 [details] [review] [SIGNED-OFF] Bug 7080: Changed casing on tab titles for fines page Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Small change for consistency. Sorry, but this patch is conflicting a little with the bug 7005 I just pushed before this one Should be easy to fix, and switch back to signed-off Created attachment 7455 [details] [review] Fix for Bug 7080 - Clean up interface on fine payment screens - Move some content out of table cells - Improve table markup with <thead>, <tbody>, and <tfoot> - Improve breadcrumb specificity on paycollect.pl - Add clearer messages for different actions (Pay selected fines, pay an individual fine, etc. - Add client-side warning to pay.pl when writing-off all - Correcting terminology: When a verb, "write off," when a noun: "Writeoff." First patch ([SIGNED-OFF] Bug 7080: Changed casing on tab titles for fines page) is just template-only case changes, so it is Passed QA and can be pushed. Second patch is rebased of previously signed off patch. Template only, adds clearer messages much in the vein of other work. Marking as Passed QA as well. it does not really matter for a frenchy like me, but I thought we've said that we wanted to use capital letters only on the 1st word. For example: "Pay fines", and not "Pay Fines". The 1st patch switches to Pay Fines. Is it inconsistent ? Is there something i've missed (it's the title of a tab, does that explain) However, i'd like to see the rule clearly explained. Owen, I think you're the best developer to explain it. Once you're made it: * i'll push the patches * i'll write the coding guideline on the wiki Thanks for your feedback There was a bug for the capitalization issue, Bug 2780, which I would favor re-opening to act as an omnibus bug report for standardizing capitalization. I agree that it would be best to follow a standard: Only the first letter of menu items, tab labels, page titles, etc. should be capitalized. (In reply to comment #15) > There was a bug for the capitalization issue, Bug 2780, which I would favor > re-opening to act as an omnibus bug report for standardizing capitalization. > > I agree that it would be best to follow a standard: Only the first letter of > menu items, tab labels, page titles, etc. should be capitalized. OK, Katrin has reopened bug 2780. Owen could you send an update for this patch removing the useless capitalized ? I've added HTML4 rule on the coding guidelines Created attachment 7514 [details] [review] Bug 7080 - Revised - Clean up interface on fine payment screens - Move some content out of table cells - Improve table markup with <thead>, <tbody>, and <tfoot> - Improve breadcrumb specificity on paycollect.pl - Add clearer messages for different actions (Pay selected fines, pay an individual fine, etc. - Add client-side warning to pay.pl when writing-off all - Correcting terminology: When a verb, "write off," when a noun: "Writeoff." Revision: Correcting tab label case according to Bug 2780 guidelines Created attachment 7596 [details] [review] [SIGNED-OFF] Bug 7080 - Revised - Clean up interface on fine payment screens - Move some content out of table cells - Improve table markup with <thead>, <tbody>, and <tfoot> - Improve breadcrumb specificity on paycollect.pl - Add clearer messages for different actions (Pay selected fines, pay an individual fine, etc. - Add client-side warning to pay.pl when writing-off all - Correcting terminology: When a verb, "write off," when a noun: "Writeoff." Revision: Correcting tab label case according to Bug 2780 guidelines Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> I checked all 4 fine tabs and performed different fine actions. The layout on all pages looks clean and correct. I tested the new client-side warning and made sure it's translatable. Labels have proper capitalization. QA comments: * valid-templates.t & translatable-templates.t OK * template changes only * tried to pay & writeoff a fine, works fine passed QA Marking this for 3.8 because there are changes in strings, will be easier for translators. It's not needed to have those changes in 3.6, as they are purely cosmetic & valid HTML & improved messages |