Description
Nick Clemens (kidclamp)
2015-03-21 17:28:46 UTC
Created attachment 37081 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Created attachment 37345 [details]
datatables
Don't look quite right. See attached.
Created attachment 37385 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before That should look better. The only way I could figure out to fix the styling was using js to alter the css but am definitely willing to update if there is a better/preferred method Sorry, this one doesn't apply. 16:25 ~/kohaclone (master % u=)$ git qa 13887 From git://git.koha-community.org/koha * branch master -> FETCH_HEAD Branch bug13887-qa set up to track remote branch master from origin. Switched to a new branch 'bug13887-qa' Bug 13887 - Add datatables to item specific holds table 37385 - Bug 13887 - Add datatables to item specific holds table Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 13887 - Add datatables to item specific holds table Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt Failed to merge in the changes. Patch failed at 0001 Bug 13887 - Add datatables to item specific holds table When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-13887---Add-datatables-to-item-specific-holds--D8nqyD.patch 16:25 ~/kohaclone (bug13887-qa *+%|AM u=)$ Created attachment 37388 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Created attachment 37411 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Hi Nick, 1/ The table does not have the tbody element 2/ Why don't you add the search box? It could be useful 3/ The "information" column should sort on due date value, don't you think? Marked as Failed QA for 1 and Discussion for 2-3. Created attachment 38294 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Created attachment 38295 [details] [review] [QA Follow up] Tab fix and "intranet-tmpl" to "[% interface %]" (In reply to Jonathan Druart from comment #8) > Hi Nick, > > 1/ The table does not have the tbody element Added - did the whitespace fixes in patch as it was adding a level > 2/ Why don't you add the search box? It could be useful I thought the search box is added by default on koha datatables, appears for me when applied, does it need to be added explicitly in this template? > 3/ The "information" column should sort on due date value, don't you think? Do you mean that should be the default sort? I liked it sorting on the hold column/ability to place a hold and that seemed the most useful. It is common for us to have only 2 or 3 items out of 50 available for hold, but we do use holds in an uncommon manner. > Marked as Failed QA for 1 and Discussion for 2-3. Setting back to signed-off but willing to change #2 or #3 if necessary Thanks Jonathan Created attachment 38309 [details] [review] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 38310 [details] [review] Bug 13887: Tab fix and "intranet-tmpl" to "[% interface %]" Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 38311 [details] [review] Bug 13887: Fix sDom to display what is needed in the table 1/ Add the information 'showing x to y of z' 2/ Remove some lost pixels Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 38312 [details] [review] Bug 13887: Move the caption into a h2 With the DT filter, the caption is between the table and the filter. It looks better with the caption as a h2 on top of the table. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Created attachment 38313 [details] [review] Bug 13887: Display the due date as a due date + sort on info The due dates should be displayed as due dates :) i.e not displayed with 23:59 On the way, this patch fixes the sort on the info column. The column is now sorted using the due dates Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Nick, Could you please test the 3 followups I have added? They improves the ergonomic of the page, The first one modify the default sDom for a better render, the second one move the caption above the filter and the last one displays the due dates correctly (and fixes the column sorting). Feel free to obsolete them if you disagree. Created attachment 38321 [details] [review] Bug 13887: Fix sDom to display what is needed in the table 1/ Add the information 'showing x to y of z' 2/ Remove some lost pixels Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 38322 [details] [review] Bug 13887: Move the caption into a h2 With the DT filter, the caption is between the table and the filter. It looks better with the caption as a h2 on top of the table. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Created attachment 38323 [details] [review] Bug 13887: Display the due date as a due date + sort on info The due dates should be displayed as due dates :) i.e not displayed with 23:59 On the way, this patch fixes the sort on the info column. The column is now sorted using the due dates Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> (In reply to Jonathan Druart from comment #17) > Could you please test the 3 followups I have added? Tested, they definitely improve the page, thanks for fixing it up! Created attachment 38369 [details] [review] [PASSED QA] Bug 13887 - Add datatables to item specific holds table This patch adds datatables functionality to the item specific holds table To test: 1 - Apply patch 2 - Attempt to place an item specific hold for a patron 3 - Note that columns are now sortable and searchable 4 - Ensure that you can place both specific and next available holds as before Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 38370 [details] [review] [PASSED QA] Bug 13887: Tab fix and "intranet-tmpl" to "[% interface %]" Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 38371 [details] [review] [PASSED QA] Bug 13887: Fix sDom to display what is needed in the table 1/ Add the information 'showing x to y of z' 2/ Remove some lost pixels Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 38372 [details] [review] [PASSED QA] Bug 13887: Move the caption into a h2 With the DT filter, the caption is between the table and the filter. It looks better with the caption as a h2 on top of the table. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 38373 [details] [review] [PASSED QA] [PASSED QA] Bug 13887: Display the due date as a due date + sort on info The due dates should be displayed as due dates :) i.e not displayed with 23:59 On the way, this patch fixes the sort on the info column. The column is now sorted using the due dates Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 38374 [details] [review] [PASSED QA] Bug 13887: Display the due date as a due date + sort on info The due dates should be displayed as due dates :) i.e not displayed with 23:59 On the way, this patch fixes the sort on the info column. The column is now sorted using the due dates Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Patches pushed to master. Thanks Nick! |