Description
Nick Clemens (kidclamp)
2018-01-03 14:07:00 UTC
Created attachment 70246 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Donwload report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected Created attachment 70643 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Donwload report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Is it possible that the lack of mention of CSV could confuse people who don't know what the text forms of CSV look like? If so, should we write «CSV (Semicolon separated)» instead of «Semicolon separated text» ? (In reply to Victor Grousset/tuxayo from comment #3) Not writing at least CSV might be right about the theoretical meaning of CSV. But if we only write "semicolon separated values" it might be possible that enough users don't realize that it's the same thing that I usually called CSV. (Opinion of someone having a better perceptive about this is needed) Also: isn't the extension still .csv anyway? So the OSes will try to open the file with a spreadsheet software. Sorry, no enough proofreading. Here is a fixed version of my last message. Minor changes but at least it doesn't burn the eyes anymore. Not writing CSV might be right about the theoretical meaning of CSV. But if we only write "semicolon separated values" it might be possible that enough users don't realize that it's the same thing that is usually called CSV. (Opinion of someone having a better perceptive about this is needed) Also: isn't the extension still .csv anyway? So the OSes will try to open the file with a spreadsheet software. Hmmm..I agree the file extension may be confusing - would you think it is better to say: XXXX Separated text (.csv file) that makes more sense to me loglically and/or we could use .txt format unless generating a csv (In reply to Nick Clemens from comment #6) > Hmmm..I agree the file extension may be confusing - would you think it is > better to say: > > XXXX Separated text (.csv file) > > that makes more sense to me logically +1 for using csv and the actual separator. After a quick web search I'm not sure "XXXX Separated text" is used elsewhere. But saying "text" in nice to convey that it's essentially text (formatted, but still text) > and/or we could use .txt format unless generating a csv Then it would be opened by a text editor instead of a spreadsheet software right? Well when the file is saved and opened by the file manager. Else (Open instead of save) the mime type transferred by Koha should make it open in the spreadsheet software. (assuming Koha says "mime type csv" or something like that) Created attachment 70977 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Donwload report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected updated text to include (.csv) and moved to an include so can be used elsewhere Comment on attachment 70977 [details] [review] Bug 19910 - Use the delimiter syspref to name download link Review of attachment 70977 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/delimiter_text.inc @@ +3,5 @@ > + [% CASE ',' %]Comma separated text (.csv) > + [% CASE '/' %]Slash separated text (.csv) > + [% CASE ';' %]Semicolon separated text (.csv) > + [% CASE '\\' %]Backslash separated text (.csv) > + [% CASE 'tabulation' %]Tab separated text (.csv) Please check if these are translatable - you might have to wrap them in spans to get them picked up. (In reply to Katrin Fischer from comment #10) > Please check if these are translatable - you might have to wrap them in > spans to get them picked up. This is what it looks like in po/en-GB-staff-prog.po after applying the patch and updating the translation: #. %1$s: SWITCH Koha.Preference('delimiter') #. %2$s: CASE '#' #. %3$s: CASE ',' #. %4$s: CASE '/' #. %5$s: CASE ';' #. %6$s: CASE '\\' #. %7$s: CASE 'tabulation' #. %8$s: END #: intranet-tmpl/prog/en/includes/delimiter_text.inc:1 #, c-format msgid "" "%s %sPound (#) separated text (.csv) %sComma separated text (.csv) %sSlash " "separated text (.csv) %sSemicolon separated text (.csv) %sBackslash " "separated text (.csv) %sTab separated text (.csv) %s " msgstr "" Nice, we are good then. Thx, Owen! Wrong bug, sorry >_< @Nick: Does the last discussion in bug 19928 affects this one? See from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19928#c29 (In reply to Victor Grousset/tuxayo from comment #14) > @Nick: Does the last discussion in bug 19928 affects this one? > > See from https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19928#c29 I suppose it does if the buttons here seem too long too. Let me describe the issue here though: 1 - Staff member without access to system preferences uses a report to get some data to load back into koha (or another system) 2 - The click 'Download as CSV' and get a *.csv file 3 - They assume the file is actually comma separated 4 - They try to load the file into Koha (or another system) which wants actual commas in a csv 5 - Things don't work 6 - They don't know why 7 - :-( Now, if you open the file in libreoffice/excel/etc. you can fix the delimiters, but sometimes you lose leading zeros etc. And it is an extra step. I just want to make sure someone who can't see the pref understands what they are getting. It could be a tooltip and not on the button It could be a note under the button We could provide selection of delimiter at export and use the pref only to determine default. What would folks prefer? >It could be a tooltip and not on the button
>It could be a note under the button
>We could provide selection of delimiter at export and use the pref only to >determine default.
I think of those suggestions none work really well with a menu type button like the one in the toolbar in reports. Am I missing something?
Thinking about it again... as this only changes the options in the menu, the button remains the same nice length. So this should be ok? So no need to change anything functionally? Let's move this bug along. Created attachment 73494 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Donwload report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 73518 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Download report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset <victor.grousset@biblibre.com> Created attachment 73592 [details] [review] Bug 19910 - Use the delimiter syspref to name download link To test: 1 - Set delimiter syspref to anything but comma 2 - Download report results as comma separated text 3 - They actually follow the syspref 4 - Apply patch 5 - Download link should now match pref selection 6 - Change pref, note link changes 7 - Verify things still work as expected Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Victor Grousset <victor.grousset@biblibre.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 73658 [details] [review] Bug 19910: Use span to make translators happy Pushed to master for 18.05, thanks to everybody involved! Pushed to stable for 17.11.06 Awesome work all! I choose not to backport to 17.05.x, to focus on major and + |