Description
Nick Clemens (kidclamp)
2017-04-18 15:02:06 UTC
Created attachment 62300 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 -Sign off Why do you want -100 to be the default? In the previous patch it was set to -120, -100 seemed to be enough - basically we want a value where librarians don't have to perform several clicks to select a year. Yes for the date of birth, but your patch modifies all the datepickers. Created attachment 62306 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 -Sign off Created attachment 62323 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 -Sign off Assume that test plan changed (birth date in staff only), works as expected. Signed-off-by: Marc Véron <veron@veron.ch> Maybe bug title should be changed to reflect the new scope of this bug (date of birth in staff client only) Why do you use the c-100;c+10 (relative to the current date) and not -120;+0 (relative to today)? It does not make sense to have years in the future here Note that the same fix should be applied to the OPAC. Ccing Owen to get his feedback. I think -100 years / +10 years from current selection is a good default. I do think it makes sense to have future dates included in the default. Sorry, I misread the changes in the patch. I thought it was a global change, and it isn't. So I guess my question is: Why not make a global change, and set a different range for specific cases like date of birth? Having more than 10 years past globally makes sense to me, but 100 seems a bit much. I am thinking of date pickers in acq/serials for example. Maybe: Global: -20, +19 Date of birth. -100, +0 But not opposed to a general change of -100, +10 either. Created attachment 65116 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years This patch makes 2 changes: 1 - Global datepicker defaulted to c-100:c+10 2 - Dateofbirth datepicker defaulted to c-100:c To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) - Make sure to check some dateofbirth datepickers 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 - Now find a dateofbirth datepicker and note default is -100:+0 6 - Sign off Created attachment 65117 [details] [review] Bug 18447 - Add changes to opac Created attachment 65118 [details] [review] Bug 18447 - Remove redundant line This line was added by bug 12342 but seems to have stopped working. These patches set default so this specific case should not be needed. Created attachment 65135 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years This patch makes 2 changes: 1 - Global datepicker defaulted to c-100:c+10 2 - Dateofbirth datepicker defaulted to c-100:c To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) - Make sure to check some dateofbirth datepickers 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 - Now find a dateofbirth datepicker and note default is -100:+0 6 - Sign off Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 65136 [details] [review] Bug 18447 - Add changes to opac Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 65137 [details] [review] Bug 18447 - Remove redundant line This line was added by bug 12342 but seems to have stopped working. These patches set default so this specific case should not be needed. Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 65140 [details] [review] [SIGNED-OFF] Bug 18447 - Datepicker only shows -10/+10 years This patch makes 2 changes: 1 - Global datepicker defaulted to c-100:c+10 2 - Dateofbirth datepicker defaulted to c-100:c To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) - Make sure to check some dateofbirth datepickers 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 - Now find a dateofbirth datepicker and note default is -100:+0 6 - Sign off Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 65141 [details] [review] [SIGNED-OFF] Bug 18447 - Add changes to opac Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 65142 [details] [review] [SIGNED-OFF] Bug 18447 - Remove redundant line This line was added by bug 12342 but seems to have stopped working. These patches set default so this specific case should not be needed. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Please look at opac-memberentry $( "#borrower_dateofbirth" ).datepicker({ yearRange: "c-120:c" The name of the control is not #dateofbirth either. Created attachment 65309 [details] [review] Bug 18447 - QA Followup - fix opac usage Created attachment 66459 [details] [review] Bug 18447 - Datepicker only shows -10/+10 years This patch makes 2 changes: 1 - Global datepicker defaulted to c-100:c+10 2 - Dateofbirth datepicker defaulted to c-100:c To test: 1 - Open some date pickers (patron entry, subscriptions, anywhere really) - Make sure to check some dateofbirth datepickers 2 - Note that default is 10 years in either direction, updated when you select 3 - Apply patch 4 - Note we now get -100 years / +10 years from current selection 5 - Now find a dateofbirth datepicker and note default is -100:+0 6 - Sign off Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66460 [details] [review] Bug 18447 - Add changes to opac Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66461 [details] [review] Bug 18447 - Remove redundant line This line was added by bug 12342 but seems to have stopped working. These patches set default so this specific case should not be needed. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 66462 [details] [review] Bug 18447 - QA Followup - fix opac usage Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Oh those SIGNED OFF labels again.. Set a date of birth for 100+ years people is not possible with these patches, that is why I suggested 120 in comment 8. (In reply to Jonathan Druart from comment #29) > Set a date of birth for 100+ years people is not possible with these > patches, that is why I suggested 120 in comment 8. Can be, with one more click. Pushed to master for 17.11, thanks to everybody involved! Looks fine to me for patron create. But years range is strange when a date of birth already exists, say "01/01/1950", years range is 1850 to 1950. I propose a follow-up to set range from current year and 120 (instead of 100) years earlier. Some people may have more than 100 years,maybe more often in the future. (Lets hope we will soon all be cyborgs ~(o_o)~ ) Created attachment 67448 [details] [review] Bug 18447: (follow-up) years range not dependent on current value Bug 18447 sets for date of birth a year range "c-100:c" meaning current year and 100 years earlier. This is good when imput is empty. But when input already contains a date, say "01/01/1950", years range is 1850 to 1950. This patch sets for date of birth a year range "-120:+0" to not be dependent on current value. Some people may have more than 100 years (maybe more often in the future) so i choose to propose maximum 120 years old. To test : 1) Go to intranet 2) Create a new patron 3) Click on date picker on "Date of birth" input 4) Look at available years in combobox, you see from 1897 to 2017 5) Edit an existing patron with for example date of birth in 1950 6) Repeat 3) and 4) 7) Go to OPAC 8) Login with a patron with for example date of birth in 1950 9) Go to "your personal details" 10) Repeate 3) and 4) (In reply to Fridolin SOMERS from comment #32) > Looks fine to me for patron create. > But years range is strange when a date of birth already exists, say > "01/01/1950", years range is 1850 to 1950. > > I propose a follow-up to set range from current year and 120 (instead of > 100) years earlier. > Some people may have more than 100 years,maybe more often in the future. > (Lets hope we will soon all be cyborgs ~(o_o)~ ) We already discussed about that, please open a new bug report if you want to modify the current behaviour. (In reply to Jonathan Druart from comment #34) > (In reply to Fridolin SOMERS from comment #32) > > Looks fine to me for patron create. > > But years range is strange when a date of birth already exists, say > > "01/01/1950", years range is 1850 to 1950. > > > > I propose a follow-up to set range from current year and 120 (instead of > > 100) years earlier. > > Some people may have more than 100 years,maybe more often in the future. > > (Lets hope we will soon all be cyborgs ~(o_o)~ ) > > We already discussed about that, please open a new bug report if you want to > modify the current behaviour. Ah indeed comment 8, sorry |