Bug 36545 adds select2 to improve the user experience when making multiple selections in item search. Bug 35746 adds the ability to reports to make multiple selections for SQL parameters. It would be nice for consistency and ux for the report multiselect to use select2.
Created attachment 167555 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to modifies multi-select parameter fields in SQL reports to use the select2 framework. If no selections are made for a multi select parameter all values will be used by default. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut. This allows the query to run when no selections are made instead of causing an error. It allows reports to be written where a multi select parameter is optional.
Created attachment 167659 [details] Run report screen Something seems wrong with the UI. This is what I'm seeing even after yarn css:build.
Thanks for looking at this Pedro! I think there is some css needed to make the little chips appear fit to the value and in line, like they do in the new item search, but I haven't been able to look much into it yet.
Created attachment 167838 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to modifies multi-select parameter fields in SQL reports to use the select2 framework. If no selections are made for a multi select parameter all values will be used by default. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut. This allows the query to run when no selections are made instead of causing an error. It allows reports to be written where a multi select parameter is optional. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 167839 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent.
(In reply to Brendan Lawlor from comment #3) > I think there is some css needed to make the little chips appear fit It was necessary to override some of the CSS inherited from fieldset.rows li.
Created attachment 167840 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to modifies multi-select parameter fields in SQL reports to use the select2 framework. If no selections are made for a multi select parameter all values will be used by default. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut. This allows the query to run when no selections are made instead of causing an error. It allows reports to be written where a multi select parameter is optional. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167841 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 169750 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to modifies multi-select parameter fields in SQL reports to use the select2 framework. If no selections are made for a multi select parameter all values will be used by default. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut. This allows the query to run when no selections are made instead of causing an error. It allows reports to be written where a multi select parameter is optional. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 169751 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent.
Rebased.
Created attachment 169818 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to modifies multi-select parameter fields in SQL reports to use the select2 framework. If no selections are made for a multi select parameter all values will be used by default. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut. This allows the query to run when no selections are made instead of causing an error. It allows reports to be written where a multi select parameter is optional. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 169819 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent. PA amended commit message: `yarn css:build` is required to test these CSS changes Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
I am unsure about the small conflict here. It's about this line: <option value="null" hidden></option> Previously changed by bug 37077. Can you please have a look and rebase yourself since you wrote the other patch too?
*** Bug 37886 has been marked as a duplicate of this bug. ***
Created attachment 171340 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to multi-select parameter fields in SQL reports. If no selections are made for a multi-select all authorised values will be used by default. This allows reports to be written where a multi select parameter is optional. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter and making single selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut Sponsored-by: CLAMS Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 171341 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent. PA amended commit message: `yarn css:build` is required to test these CSS changes Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 171342 [details] [review] Bug 37050: (follow-up) Add select2 to SQL reports multiselect Rebase for changes made on bug 37077 This patch removes some javascript in favor of an update to the template. The default value for multi select parameter's is a newline separated list so that report results are not limited when no selections are made.
Created attachment 171957 [details] [review] Bug 37050: Add select2 to SQL reports multi select This patch adds select 2 to multi-select parameter fields in SQL reports. If no selections are made for a multi-select all authorised values will be used by default. This allows reports to be written where a multi select parameter is optional. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter and making single selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut Sponsored-by: CLAMS Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 171958 [details] [review] Bug 37050: (follow-up) CSS and configuration tweak This follow-up tweaks the Select2 CSS so that the selected choices don't display with 100% width. The patch also removes the "closeOnSelect: false" configuration from Select2. In discussing this option for Bug 36545 more people preferred the default behavior and I think we should be consistent. PA amended commit message: `yarn css:build` is required to test these CSS changes Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Created attachment 171959 [details] [review] Bug 37050: (follow-up) Add select2 to SQL reports multiselect Rebase for changes made on bug 37077 This patch removes some javascript in favor of an update to the template. The default value for multi select parameter's is a newline separated list so that report results are not limited when no selections are made. Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Taking another look here
Observations: It feels a bit weird to allow for an empty selection (causing an empty box with an x), and have that empty option behave the same as if no option was selected at all. Should we just not show an empty option in this case? The user can deselect any selections and submit it empty anyway. Found the following error: A) Follow steps 1 and 2 of the test plan, submit the empty form -> all good B) Follow steps 1 and 2 of the test plan, pick a single library, then remove, submit the empty form -> BOOM This may have been caused by bug 35746 and not the patches here. For the reasons above I'm not comfortable putting this back to PQA, but I don't think they're strong enough reasons to FQA either (1 is an observation, the other a bug that I don't relates to patches here). @Brendan, what do you think of the comments above?
Hi Pedro, thanks for finding these issues! (In reply to Pedro Amorim from comment #23) > Observations: > It feels a bit weird to allow for an empty selection (causing an empty box > with an x), and have that empty option behave the same as if no option was > selected at all. Should we just not show an empty option in this case? The > user can deselect any selections and submit it empty anyway. I hadn't noticed that the hidden select option is selectable in the select2 dropdown. I agree this should be fixed. It seems to be that select2 is not respecting the hidden property on that first option for some reason. > Found the following error: > A) Follow steps 1 and 2 of the test plan, submit the empty form -> all good > B) Follow steps 1 and 2 of the test plan, pick a single library, then > remove, submit the empty form -> BOOM The template fills all the codes into the hidden option and then every time the user updates selections the value of the hidden option is updated. This error is happening because of an edge case when everything is deselected the value is becoming an empty string when it should revert to a newline delimited list of all codes. I will come back to this and submit a new patch as soon as I can. Thanks!
@Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you please review and file new bugs if needed?
(In reply to Katrin Fischer from comment #25) > @Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you > please review and file new bugs if needed? Hi Katrin! I added "additional_work_needed" to remind me to come back to this one. Should I have just set it to Failed QA instead? I intend to work on this but haven't had the time recently.
(In reply to Brendan Lawlor from comment #26) > (In reply to Katrin Fischer from comment #25) > > @Pedro and @Brendan: Hi there, reviewing "additional_work_needed" - can you > > please review and file new bugs if needed? > > > Hi Katrin! > > I added "additional_work_needed" to remind me to come back to this one. > Should I have just set it to Failed QA instead? > > I intend to work on this but haven't had the time recently. Hah! That was my mistake, we usually use the additional_work_needed for bugs already pushed that still need something done. Failed QA or back to Assigned is fine if you need to work on it still.
Created attachment 177320 [details] [review] Bug 37050: Add select2 to SQL reports multiselect This patch adds select 2 to multi-select parameter fields in SQL reports. If no selections are made for a multi-select all authorised values will be used by default. This allows reports to be written where a multi select parameter is optional. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter and making single selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut Sponsored-by: CLAMS
Created attachment 179669 [details] [review] Bug 37050: Add select2 to SQL reports multiselect This patch adds select 2 to multi-select parameter fields in SQL reports. If no selections are made for a multi-select all authorised values will be used by default. This allows reports to be written where a multi select parameter is optional. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter and making single selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut Sponsored-by: CLAMS
rebased for main
Created attachment 179728 [details] [review] Bug 37050: Add select2 to SQL reports multiselect This patch adds select 2 to multi-select parameter fields in SQL reports. If no selections are made for a multi-select all authorised values will be used by default. This allows reports to be written where a multi select parameter is optional. To test: 1. Apply patch 2. Go to Reports and create a report from SQL select * from items where homebranch in <<Libraries|branches:in>> and itype in <<Item type|itemtypes:in>> 3. Run the report making multiple selections for each parameter and making single selections for each parameter 4. Run the report making multiple selections for one parameter and no selections in another parameter 5. Check that the results are as you would expect 6. On the results page click 'Show SQL code' 7. Confirm that the codes are included according to your selections 8. Note that when no selections are made for a parameter all codes are used by defaut Sponsored-by: CLAMS Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Ready for Pedro ;-)