Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the word order of some of the toast messages #12446

Merged
merged 4 commits into from
Mar 26, 2023

Conversation

snowtimeglass
Copy link
Contributor

Pull Request template

Purpose / Description

Anki Desktop uses the following word order pattern on some toast messages, for example, Card suspended.,

  1. Subject
  2. omission of be Verb
  3. Past participle

but it isn't used on some strings of AnkiDroid, for example, Suspended card.
It is probably appropriate to use Anki Desktop's word order pattern on AnkiDroid as well.

image

Fixes

n/a

Approach

Change the strings on 02-strings.xml

How Has This Been Tested?

on my physical device
imageimageimage
imageimage
image

Learning (optional, can help others)

n/a

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@github-actions
Copy link
Contributor

Message to maintainers, this PR contains strings changes.

  1. Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
  2. Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

@BrayanDSO
Copy link
Member

Actually, we have access to Anki Desktop strings, so we can reuse them directly. I'm on my phone right now, so I can't link the exact implementation on the moment, but if I recall correctly, it is on col.TR. So that is an option.

As the current PR only changes the English translation, I'd approve it, but I would like to see if the approach of reusing the Anki Desktop strings directly wouldn't be better

@mikehardy
Copy link
Member

Indeed - here's an example usage https://github.com/ankidroid/Anki-Android/pull/11740/files#diff-03de22cd624f1cb4006ae0c6622ec035f598d779dd1dfb64e2207f0a1824933cR42

And I just logged a couple items with regard to backend translation - it's all pretty new for us (#12452 + ankidroid/Anki-Android-Backend#235)

@dae
Copy link
Contributor

dae commented Sep 20, 2022

You can also use CollectionManager.TR(), which doesn't require an open collection or usage of withCol

@dae
Copy link
Contributor

dae commented Sep 20, 2022

Here's how you could do it:

diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
index a13e4e352..c122b8bd8 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
@@ -50,6 +50,7 @@ import com.drakeet.drawer.FullDraggableContainer
 import com.google.android.material.snackbar.Snackbar
 import com.ichi2.anim.ActivityTransitionAnimation
 import com.ichi2.anim.ActivityTransitionAnimation.getInverseTransition
+import com.ichi2.anki.CollectionManager.TR
 import com.ichi2.anki.CollectionManager.withCol
 import com.ichi2.anki.UIUtils.showThemedToast
 import com.ichi2.anki.cardviewer.*
@@ -908,6 +909,15 @@ abstract class AbstractFlashcardViewer :
         }
     }
 
+    private fun showSnackbarWithUndoButtonText(
+        text: String,
+        duration: Int = Snackbar.LENGTH_SHORT
+    ) {
+        showSnackbarAboveAnswerButtons(text, duration) {
+            setAction(R.string.undo) { undo() }
+        }
+    }
+
     private fun getRecommendedEase(easy: Boolean): Int {
         return try {
             when (answerButtonCount) {
@@ -1648,7 +1658,7 @@ abstract class AbstractFlashcardViewer :
 
     internal fun buryCard(): Boolean {
         return dismiss(BuryCard(currentCard!!)) {
-            showSnackbarWithUndoButton(R.string.buried_card)
+            showSnackbarWithUndoButtonText(TR.studyingCardsBuried(1))
         }
     }
 

@snowtimeglass
Copy link
Contributor Author

Thank you all for the information and advice. I'll try :-D

@snowtimeglass
Copy link
Contributor Author

I would like to see if the approach of reusing the Anki Desktop strings directly wouldn't be better

@BrayanDSO I've tried the approach with CollectionManager.TR() as @dae taught me.
image

My concern is that the approach makes inconsistency about punctuation between the reused messages and the other ones, at least for the moment. In general, the messages of AnkiDroid end without period as follows
image
while ones of Anki Desktop generally (maybe) end with it.
image
image

@mikehardy
Copy link
Member

Using Anki translations is something I prefer as it represents a more "shared work" future (between Anki Desktop and AnkiDroid), which may also be read as "less work per contributor" and is thus more sustainable long-term, even if there is punctuation inconsistency.

@BrayanDSO
Copy link
Member

Anyone can remove the dot on Poonton (Anki desktop translating platform) later. Sharing translations is the best path for consistency

@snowtimeglass
Copy link
Contributor Author

snowtimeglass commented Sep 25, 2022

I see. Then, I'll continue this approach without so much concern about temporary inconsistency of punctuation.

About the approach, I have one question.

diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
-            showSnackbarWithUndoButton(R.string.buried_card)
+            showSnackbarWithUndoButtonText(TR.studyingCardsBuried(1))

For example, if the above replacement is done, should I delete
<string name="buried_card">Card buried</string>
in AnkiDroid/src/main/res/values/02-strings.xml?

@BrayanDSO
Copy link
Member

If the string isn't somewhere else, you can delete it. Actually, Lint will force will to delete it on this case.

@snowtimeglass
Copy link
Contributor Author

Thanks. Then, I guess I'd better leave it to Lint Debug to check if the string isn't somewhere else.

@snowtimeglass
Copy link
Contributor Author

After Bury note action is done, a message, for example, 2 cards buried., is shown on Anki Desktop.
image

In order to reproduce it on AnkiDroid with Anki Desktop strings, what should be done? Filling in some variable in the following parentheses?

internal fun buryNote(): Boolean {
        return dismiss(BuryNote(currentCard!!)) {
            showSnackbarWithUndoButtonText(TR.studyingCardsBuried(●))

image

@BrayanDSO
Copy link
Member

On methods that operate on a note, you can use currentCard!!.note().numberOfCards(). If it operates on a card, passing 1 should be fine

@snowtimeglass
Copy link
Contributor Author

I see. Thanks a lot!

@snowtimeglass
Copy link
Contributor Author

@dae On Anki Desktop, the actions of Bury and Suspend produce messages whose styles are different from each other as follows. Is it intentional?

Action --> Message

Bury Card --> 1 card buried.
Bury Note --> 2 cards buried. (the case of a note with 2 cards)

Suspend Card --> Card suspended.
Suspend Note --> Note suspended.

@mikehardy
Copy link
Member

@dae good question for you just above ☝️

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label Oct 13, 2022
@dae
Copy link
Contributor

dae commented Oct 14, 2022

Sorry, lost track of this. I think the difference is just because they were added at different times by different people. Suspend card/note came earlier and is only used in the review screen; the bury message was added later, and including the number was likely because it can be used in both the reviewer and browse screen (even though the browse screen doesn't currently provide a bury option, it may in the future).

@snowtimeglass
Copy link
Contributor Author

snowtimeglass commented Oct 15, 2022

@dae @mikehardy thanks. Then, I suppose the following hybrid style might be good.

Action --> Message

Bury Card --> Card buried.
Bury Note --> n cards buried. (n≧2) ; 1 card buried. (the case of executing Bury Note gesture on a card which has no siblings. On AnkiDroid, Bury Note item itself on action menu isn't shown for the card)

Same for Suspend

Suspend Card --> Card suspended.
Suspend Note --> n cards suspended. (n≧2) ; 1 card suspended. (the case of executing Suspend Note gesture on a card which has no siblings. On AnkiDroid, Suspend Note item itself on action menu isn't shown for the card)

May I continue working in this direction?

@BrayanDSO
Copy link
Member

Fine with me

@snowtimeglass
Copy link
Contributor Author

snowtimeglass commented Oct 23, 2022

On Pontoon of Anki, the string of Card buried seems to exist only in AnkiMobile resource.
image

What approach would be preferable?
Using the string of AnkiMobile in some way?
Requesting creation of that string in Anki Desktop resource?
Just replacing
<string name="buried_card">Buried card</string>
with
<string name="buried_card">Card buried</string> in AnkiDroid/src/main/res/values/02-strings.xml?
Or replacing it with
<string name="Card_buried">Card buried</string>?

@BrayanDSO
Copy link
Member

If we can use the mobile translation with CollectionManager.TR, use it

else if we have an AnkiDroid string equivalent to Card buried/Buried card, use it

else, create a new one on AnkiDroid

@dae
Copy link
Contributor

dae commented Oct 24, 2022

The AnkiMobile translations are not easily usable. It's possible to migrate individual translations from mobile→desktop, but it's currently it's a bit of a pain to do so I'm afraid.

@snowtimeglass
Copy link
Contributor Author

Thank you, two. Then, rewriting the following related string on AnkiDroid is currently a handy way, I guess.

<string name="buried_card">Buried card</string>

@snowtimeglass

This comment was marked as resolved.

snowtimeglass added a commit to snowtimeglass/Anki-Android that referenced this pull request Jan 13, 2023
Reconstruct the strings for the feedback-toast message to Reviewer actions

Basically, use strings on Anki Desktop instead of the current ones. Create new ones as needed.
For Bury and Suspend, I set the style described in the following comment as a goal:
  ankidroid#12446 (comment)

------------------------------

- Replace `<string name="buried_card">Card buried</string>`
        with `<string name="card_buried">Card buried.</string>`

- Remove `<string name="buried_note">Note buried</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="deleted_note">Note deleted</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="suspended_card">Card suspended</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Replace `<string name="suspended_note">Note suspended</string>`
        with the following code:
                `<plurals name="note_suspended"
                        <item quantity="one">%d card suspended.</item>
                        <item quantity="other">%d cards suspended.</item>
                 </plurals>`

------------------------------
@snowtimeglass
Copy link
Contributor Author

The results of the last commits are as follows. I would deeply appreciate it if you would give me a review and some advice.
image
image
image
image

@github-actions
Copy link
Contributor

Hello 👋, this PR has been opened for more than 1 month with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Feb 21, 2023
@david-allison david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author Stale labels Feb 21, 2023
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm going to rebase this one on main, as it's been stale for a while and some strings need fixing

Comment on lines 381 to 391
<string name="card_buried">Card buried.</string>
<plurals name="note_suspended"
<item quantity="one">%d card suspended.</item>
<item quantity="other">%d cards suspended.</item>
</plurals>
Copy link
Member

@david-allison david-allison Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up to either lint these or change Anki Desktop to remove the period (depends on dae's thoughts, not worthwhile to ping for something minor until we get our ducks in order)

@david-allison david-allison self-assigned this Feb 21, 2023
@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Feb 21, 2023
Change the word order of some of the toast messages

"[Subject] (omission of [be Verb]) [Past participle]" word order is used in Anki Desktop and AnkiDroid, but it isn't on some strings of AnkiDroid

before change  -->   after change
-------------------------------------------------
"Buried card" --> "Card buried"
"Buried note" --> "Note buried"
"Deleted note" --> "Note deleted"
"Suspended card" --> "Card suspended"
"Suspended note" --> "Note suspended"
"Created deck"  --> "Deck created"
"Renamed deck"  --> "Deck renamed"
"Copied debug information to clipboard" --> "Debug information was copied to clipboard" (no omission of be Verb)
-------------------------------------------------
Reconstruct the strings for the feedback-toast message to Reviewer actions

Basically, use strings on Anki Desktop instead of the current ones. Create new ones as needed.
For Bury and Suspend, I set the style described in the following comment as a goal:
  ankidroid#12446 (comment)

------------------------------

- Replace `<string name="buried_card">Card buried</string>`
        with `<string name="card_buried">Card buried.</string>`

- Remove `<string name="buried_note">Note buried</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="deleted_note">Note deleted</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="suspended_card">Card suspended</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Replace `<string name="suspended_note">Note suspended</string>`
        with the following code:
                `<plurals name="note_suspended"
                        <item quantity="one">%d card suspended.</item>
                        <item quantity="other">%d cards suspended.</item>
                 </plurals>`

------------------------------
- For feedback-toast message to Reviewer actions, reuse strings of Anki Desktop
- Use new strings instead of ones of Anki Desktop, as needed
old: about_ankidroid_successfully_copied_debug
new: about_ankidroid_successfully_copied_debug_info
@snowtimeglass
Copy link
Contributor Author

Thank you so much for taking the trouble to do the review and the works!

Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@lukstbit lukstbit added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Mar 25, 2023
@mikehardy
Copy link
Member

I'm clearing strings out now then will merge the strings-related stack of PRs queued up

@mikehardy mikehardy merged commit f8f0146 into ankidroid:main Mar 26, 2023
mikehardy pushed a commit that referenced this pull request Mar 26, 2023
Reconstruct the strings for the feedback-toast message to Reviewer actions

Basically, use strings on Anki Desktop instead of the current ones. Create new ones as needed.
For Bury and Suspend, I set the style described in the following comment as a goal:
  #12446 (comment)

------------------------------

- Replace `<string name="buried_card">Card buried</string>`
        with `<string name="card_buried">Card buried.</string>`

- Remove `<string name="buried_note">Note buried</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="deleted_note">Note deleted</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="suspended_card">Card suspended</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Replace `<string name="suspended_note">Note suspended</string>`
        with the following code:
                `<plurals name="note_suspended"
                        <item quantity="one">%d card suspended.</item>
                        <item quantity="other">%d cards suspended.</item>
                 </plurals>`

------------------------------
@github-actions github-actions bot added this to the 2.16 release milestone Mar 26, 2023
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Mar 26, 2023
@snowtimeglass snowtimeglass deleted the word_order branch March 26, 2023 12:26
@github-actions
Copy link
Contributor

Hi there @snowtimeglass! This is the OpenCollective Notice for PRs merged from 2023-03-01 through 2023-03-31

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please note that GSoC contributions are okay for this process. Our philosophy is that our users have donated to AnkiDroid for all contributions. The only PRs that will not go through the OpenCollective process are ones directly related to am accepted GSoC project from a selected participant, since those receive a stipend from GSoC itself.

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

KendallPark pushed a commit to KendallPark/Anki-Android that referenced this pull request May 30, 2023
Reconstruct the strings for the feedback-toast message to Reviewer actions

Basically, use strings on Anki Desktop instead of the current ones. Create new ones as needed.
For Bury and Suspend, I set the style described in the following comment as a goal:
  ankidroid#12446 (comment)

------------------------------

- Replace `<string name="buried_card">Card buried</string>`
        with `<string name="card_buried">Card buried.</string>`

- Remove `<string name="buried_note">Note buried</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="deleted_note">Note deleted</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Remove `<string name="suspended_card">Card suspended</string>`
   (Instead of the string, its counterpart on Anki Desktop will work on the toast.)

- Replace `<string name="suspended_note">Note suspended</string>`
        with the following code:
                `<plurals name="note_suspended"
                        <item quantity="one">%d card suspended.</item>
                        <item quantity="other">%d cards suspended.</item>
                 </plurals>`

------------------------------
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants