| Summary: | Reports row limit should change upon option selection | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
| Component: | Reports | Assignee: | Aleisha Amohia <aleisha> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | jonathan.druart, kyle, veron |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | Sponsored | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Bug 16389: Reports row limit should change upon option selection
Bug 16389: Reports row limit should change upon option selection Bug 16389: Reports row limit should change upon option selection |
||
|
Description
Aleisha Amohia
2016-04-29 03:00:32 UTC
Created attachment 50978 [details] [review] Bug 16389: Reports row limit should change upon option selection To test: 1) Run a report 2) Confirm there is no 'Update' button next to the 'Rows per page:' dropdown 3) Change the limit (i.e. to 10) 4) Confirm the page updates itself Sponsored-by: Catalyst IT Created attachment 50991 [details] [review] Bug 16389: Reports row limit should change upon option selection To test: 1) Run a report 2) Confirm there is no 'Update' button next to the 'Rows per page:' dropdown 3) Change the limit (i.e. to 10) 4) Confirm the page updates itself Sponsored-by: Catalyst IT Signed-off-by: Marc Véron <veron@veron.ch> Aleisha,
The patch works as advertise, but I would prefer to see more descriptive/useful name for the ids.
For instance, the id on the select is not needed.
I'd say we could change the jQuery selector instead to $('select.limit').
On the same way, we could search for the form using the parents() selector, but I think it's better to select the form by id. But 'limitselect' is not a correct description for the form element.
Created attachment 51021 [details] [review] Bug 16389: Reports row limit should change upon option selection To test: 1) Run a report 2) Confirm there is no 'Update' button next to the 'Rows per page:' dropdown 3) Change the limit (i.e. to 10) 4) Confirm the page updates itself Sponsored-by: Catalyst IT Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> After a chat with Owen, your patch looks good. Pushed to master for the 16.05 release! Thanks Aleisha! |