Change the mail subject when sharing a file to something more consise by ducalex · Pull Request #51 · grote/BlitzMail

ducalex · 2021-08-31T19:39:46Z

This is related to #28.

I went for the simplest possible, and while testing I found that a prefix was more intuitive than a suffix. We could even remove the prefix IMHO, the filename seems to be enough.

But I'd be happy to change the format to whatever is preferred, as were suggested before!

Examples of the new subjects:

Single file: File: MY_FILE.png
Two files: Files: MY_FILE.png and MY_FILE2.png
More files: Files: MY_FILE.png and 3 more

Examples of the new subjects: Single file: `File: MY_FILE.png` Two files: `Files: MY_FILE.png and MY_FILE2.png` More files: `Files: MY_FILE.png and 3 more`

ducalex · 2021-08-31T19:46:28Z

Thinking about it we could even ditch the two files format and only keep:
Single file: File: MY_FILE.png
2 or more files: Files: MY_FILE.png and 1 more

grote · 2021-09-01T07:12:03Z


+    <string name="files_shared_1">File: %s</string>
+    <string name="files_shared_2">Files: %s and %s</string>
+    <string name="files_shared_3">Files: %s and %d more</string>


From my side, we can remove the File(s): prefix. However, the last string needs to be a quantity string, because different languages have different ways to say 1 more, 2 more, 3 more, etc.

I tested having a suffix first, then tried prefix and realized it made it so much clearer that it's an attachment at first glance while browsing emails.

But can you let me know your preferred format for 1, 2, and 3+ files? I'll update this pull request to exactly match your choice.

If you think we need the prefix, I am fine with it. I just think that files_shared_3 needs to be a quantity string.

I've replaced files_shared_1/2/3 by a single plural string. It gives us a little less flexibility (we lose files_shared_2 because no quantity expresses 2 reliably) but it makes the code simpler and presumably easier to translate, I can see that now!

grote · 2021-09-02T12:35:22Z

+    <plurals name="files_shared">
+        <item quantity="one">File: %3$s</item>
+        <item quantity="other">Files: %3$s and %2$d more</item>
+    </plurals>


I think you can add this to strings.xml which makes it easier for translators and us to maintain.

Also, why %3$s and %2$d ? where's %1$s

Also, why %3$s and %2$d ? where's %1$s

%1$s isn't used in English but according to the documentation, some languages like Korean, only have the "other" quantity which would require the total number of files. I've moved the translation to strings.xml and I've added a comment to describe the 3 arguments.

Thanks! I am just confused about %1$s. If it isn't used in English why would it be in other languages? I've never seen string placeholders not being used in the source language before.

I've removed it.

I've separated the translations for single attachment and multiple attachment, which looking back is probably what you were suggesting initially.

That way it should work in all languages and only the subject with N more is puralized.

grote · 2021-09-07T07:14:45Z

-			String num = String.valueOf(jMail.getJSONArray("attachments").length());
-			jMail.put("subject", num + " " + getString(R.string.files_shared) + " " + getString(R.string.app_name));
+			JSONArray files = jMail.getJSONArray(MAIL_ATTACHMENTS);
+			String filename = files.getJSONObject(0).getString("filename");


Thanks for using the MAIL_ATTACHMENTS constant here. Do we also have one for filename and subject?

Good point. There was one for MAIL_SUBJECT which I just updated but there isn't one for filename.

I'd be happy to define a scheme or constants for attachments but are you sure this should be done in this pull request?

Can be in another one, just brought it up since you started to use constants.

…iteral

grote · 2021-09-08T07:39:24Z

+			}
+			if (files.length() == 1) {
+				jMail.put(MAIL_SUBJECT, getString(R.string.subject_single_file, filename));
+			} else {


Something I notice just now. Can it happen that files.length() == 0 for some reason?

I couldn't find an easy way to get 0 because even if an attachment fails (eg IOException) it would still be put in attachments.
However I have made changes to hopefully cover any possibility of attachments being empty or null or missing from the jMail object.

(I've had some commit issues hopefully my changes went through.)

Change the mail subject when sharing a file to something more consise

d7edb37

Examples of the new subjects: Single file: `File: MY_FILE.png` Two files: `Files: MY_FILE.png and MY_FILE2.png` More files: `Files: MY_FILE.png and 3 more`

grote reviewed Sep 1, 2021

View reviewed changes

Switched to quantity string for files_shared

39a8adf

ducalex force-pushed the file-subject branch from ce87b5d to 39a8adf Compare September 2, 2021 12:24

grote requested changes Sep 2, 2021

View reviewed changes

ducalex force-pushed the file-subject branch from 95b1a2e to 2360c21 Compare September 2, 2021 22:04

Moved file_shared plurals to strings.xml

8aa49c9

ducalex force-pushed the file-subject branch 2 times, most recently from 73fae1d to 339b67e Compare September 3, 2021 02:00

Separated translations for single/multiple attached files

aabd1d4

ducalex force-pushed the file-subject branch from 339b67e to aabd1d4 Compare September 3, 2021 02:02

grote reviewed Sep 7, 2021

View reviewed changes

grote approved these changes Sep 7, 2021

View reviewed changes

SendActivity: Use MAIL_SUBJECT constant instead of "subject" string l…

ffd5d27

…iteral

grote reviewed Sep 8, 2021

View reviewed changes

ducalex force-pushed the file-subject branch 2 times, most recently from 221c251 to 92f5d98 Compare September 9, 2021 01:14

SendActivity: Improve error handling with attachments

acccc1a

ducalex force-pushed the file-subject branch from 92f5d98 to acccc1a Compare September 9, 2021 01:17

Conversation

ducalex commented Aug 31, 2021

Uh oh!

ducalex commented Aug 31, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ducalex Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grote Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grote Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ducalex Sep 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ducalex commented Aug 31, 2021 •

edited

Loading

ducalex Sep 1, 2021 •

edited

Loading

grote Sep 1, 2021 •

edited

Loading

grote Sep 7, 2021 •

edited

Loading

ducalex Sep 9, 2021 •

edited

Loading