It appears that the new Days mode value in the circulation conditions is not explained and the values are rather cryptic - especially if you are supposed to translate them: Calendar DateDue Days DayWeek Default There are references in the code, but I don't see where they show up visibly. And the DaysMode system preference doesn't use the codes so you can't make the connection there easily. Might be missing something, but glad to be told!
DateDue means "next opening day"...?
Days is supposed to mean 'ignore the calendar' but if you look at translations that won't work well.
(In reply to Katrin Fischer from comment #2) > Days is supposed to mean 'ignore the calendar' but if you look at > translations that won't work well. Especially since this string is used in lots of places :(
Created attachment 113739 [details] [review] Bug 27026: (bug 24159 follow-up) Add a tooltip to explain the values of "Days mode" There was a tooltip for values of the existing rules, but not the only you were editing. Now there is the same tooltip (title) on the option elements of the dropdown list.
Would this patch works for you? We can even have it post string freeze as the strings already existed.
Hi Jonathan, thx for taking a look at this! The titles will certainly help, but I feel like we should really change the options offered. I know they are derived from the database values for the syspref, but I feel they make not much sense, especially when people try to translate them (and they will, especially since calendar appears in multiple places like the tools start page). <option value="" title="Use the system preference 'useDaysMode' as a default value">Default</option> <option value="Calendar" title="Use the calendar to skip days the library is closed">Calendar</option> <option value="Datedue" title="Use the calendar to push the due date to the next open day">Datedue</option> <option value="Days" title="Ignore the calendar">Days</option> <option value="Dayweek" title="Use the calendar to push the due date to the next open matching weekday for weekly loan periods, or the next open day otherwise">Dayweek</option> Why not use something more speaking? Default Skip closed days Next open day Same week day
Created attachment 113770 [details] [review] Bug 27026: Use meaningful descriptions
Created attachment 113807 [details] [review] Bug 27026: (bug 24159 follow-up) Add a tooltip to explain the values of "Days mode" There was a tooltip for values of the existing rules, but not the only you were editing. Now there is the same tooltip (title) on the option elements of the dropdown list. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113808 [details] [review] Bug 27026: Use meaningful descriptions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 113912 [details] [review] Bug 27026: (bug 24159 follow-up) Add a tooltip to explain the values of "Days mode" There was a tooltip for values of the existing rules, but not the only you were editing. Now there is the same tooltip (title) on the option elements of the dropdown list. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 113913 [details] [review] Bug 27026: Use meaningful descriptions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
As a step towards QA, here is another signoff. And the QA script passes.
These look reasonable to me and are an improvement.. I must admit I still get confused by 'Next open day' vs 'Skip closed days'.. they seem equivalent in description to me... what do they do differently?
Says you have: Today is Wed Nov 4th, Sat and Sun are closed, as well as Nov 11th November 2020 Su Mo Tu We Th Fr Sa 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 * "Calendar - Use the calendar to skip days the library is closed": With today + 7 days you will get 16th (skipping 7, 8, 11, 14, 15) * "Use the calendar to push the due date to the next open day": With today + 7 days you will get 12th: today + 7 gave you 11th, which is a closed day, so you get the day after.
Jonathan beat me to it :) Next opening day counts all calendar days and just shifts to next opening day at the end, if it hits a closed day. Skip closed days doesn't count them at all for the loan period (thus always landing on an opening day I think).
This is PQA.
Created attachment 113987 [details] [review] Bug 27026: (bug 24159 follow-up) Add a tooltip to explain the values of "Days mode" There was a tooltip for values of the existing rules, but not the only you were editing. Now there is the same tooltip (title) on the option elements of the dropdown list. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 113988 [details] [review] Bug 27026: (bug 24159 follow-up) Add a tooltip to explain the values of "Days mode" There was a tooltip for values of the existing rules, but not the only you were editing. Now there is the same tooltip (title) on the option elements of the dropdown list. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Bug 27026: Use meaningful descriptions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Patches squashed.
Pushed to master for 20.11, thanks to everybody involved!
Missing dependencies, not backported to 20.05.x