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

AnkiDroid to use flag names like anki #16242

Merged
merged 1 commit into from
May 19, 2024

Conversation

Giyutomioka-SS
Copy link
Contributor

@Giyutomioka-SS Giyutomioka-SS commented Apr 21, 2024

Purpose / Description

AnkiDroid to use Flag names like Anki

Fixes

How Has This Been Tested?

On Realme 6 & Emulator

Screen Recording

video_2024-04-22_01-29-40.mp4

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

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,

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 great! Needs to handle errors, other than that, the other comment is a nitpick and this is good to go

AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt Outdated Show resolved Hide resolved
AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt Outdated Show resolved Hide resolved
@criticalAY
Copy link
Contributor

'byte[][] net.ankiweb.rsdroid.NativeMethods.openBackend(byte[])'
Unit test are likely to fail again

@Giyutomioka-SS
Copy link
Contributor Author

@david-allison, Unit tests are failing again. What should be done?

@Aditya13s
Copy link
Contributor

@Giyutomioka-SS Remove the code or resource which are not in use.

@Giyutomioka-SS
Copy link
Contributor Author

@Giyutomioka-SS Remove the code or resource which are not in use.

Already did

@david-allison david-allison added Help Wanted Requesting Pull Requests from volunteers Needs Review labels Apr 22, 2024
@david-allison
Copy link
Member

Ping me in 24h, feel free to pick up another issue in the meantime

@david-allison david-allison self-assigned this Apr 22, 2024
@david-allison david-allison added Needs Author Reply Waiting for a reply from the original author and removed Help Wanted Requesting Pull Requests from volunteers labels Apr 22, 2024
@Giyutomioka-SS
Copy link
Contributor Author

Ping me in 24h, feel free to pick up another issue in the meantime

@david-allison

@david-allison
Copy link
Member

@Giyutomioka-SS with apologies, my laptop died today.

Try our Discord for now, I'll be a while to get back to normal

@Giyutomioka-SS
Copy link
Contributor Author

Giyutomioka-SS commented Apr 25, 2024

@david-allison please don’t apologize, machines can give up anytime, and i always keep a tab on anki discord just don’t talk much😅

@david-allison
Copy link
Member

Feel free to ping the channel for help/a review, it's what we're here for!

@mikehardy
Copy link
Member

Not sure why, but the backend is not being found?


        Caused by:
        java.lang.ExceptionInInitializerError: Exception java.lang.UnsatisfiedLinkError: 'byte[][] net.ankiweb.rsdroid.NativeMethods.openBackend(byte[])' [in thread "ForkJoinPool-1-worker-1"]
            at net.ankiweb.rsdroid.NativeMethods.openBackend(Native Method)
            at net.ankiweb.rsdroid.Backend.<init>(Backend.kt:79)
            at net.ankiweb.rsdroid.BackendFactory.getBackend(BackendFactory.kt:39)
            at net.ankiweb.rsdroid.BackendFactory.getBackend$default(BackendFactory.kt:37)
            at com.ichi2.anki.CollectionManager.ensureBackendInner(CollectionManager.kt:183)
            at com.ichi2.anki.CollectionManager.access$ensureBackendInner(CollectionManager.kt:39)
            at com.ichi2.anki.CollectionManager$getBackend$1$1.invoke(CollectionManager.kt:132)
            at com.ichi2.anki.CollectionManager$getBackend$1$1.invoke(CollectionManager.kt:132)
            at com.ichi2.anki.CollectionManager$withQueue$2.invokeSuspend(CollectionManager.kt:86)
            at _COROUTINE._BOUNDARY._(CoroutineDebugging.kt:42)
            at com.ichi2.anki.CollectionManager$getBackend$1.invokeSuspend(CollectionManager.kt:132)

I looked at the commit history - it should be squashed - no reason for 5 commits - so I recommend learning how git rebase works and perhaps doing a git rebase -i HEAD~5 then changing pick on commits 2 through 5 to squash and after getting your local branch cleaned up like that so it is a single commit doing a git push --force-with-lease to push your cleaned-up local branch back out to github for this PR - that will kick off CI again and perhaps the linker problem will be solved?

If it is not solved by that, then I would also do a git add remote upstream git@github.com:ankidroid/Anki-Android, then a git fetch upstream, then a git rebase upstream/main on your branch to pull in all upstream changes to make sure the backend dependency is correct and again git push --force-with-lease to push those changes back out to github so the PR is updated. That will run CI again and hopefully the problem will be solved?

@criticalAY
Copy link
Contributor

@david-allison @Giyutomioka-SS It is blocked until I come up with the right code to handle the flag names, as the previous commit I made related to flag names was reverted

@BrayanDSO
Copy link
Member

It is blocked until I come up with the right code to handle the flag names,

I don't see a reason for blocking this one. It doesn't matter which PR goes first, as long as is works and doesn't break CI

@criticalAY
Copy link
Contributor

This PR uses a method which was reverted to fix the CI failure and that is the reason for the conflicts, but if you intend to replace strings only then go ahead, @Giyutomioka-SS remember to find and replace the strings at all places now

Copy link
Contributor

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. 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 May 13, 2024
@Giyutomioka-SS
Copy link
Contributor Author

Giyutomioka-SS commented May 13, 2024

@BrayanDSO @david-allison @criticalAY Could you please review this once?

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.

This is no longer using the backend

displayName should be a method

@criticalAY
Copy link
Contributor

On my phone but there should be a better way than the current code to achieve the same result @Giyutomioka-SS instead of overriding it for all the strings

@Giyutomioka-SS
Copy link
Contributor Author

On my phone but there should be a better way than the current code to achieve the same result @Giyutomioka-SS instead of overriding it for all the strings

Unit tests are failing if I am using TR this way in Flag.kt should I revert this to the previous commit it works fine.

Copy link
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

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

Will approve after the blue flag name is corrected

AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt Outdated Show resolved Hide resolved
AnkiDroid/src/main/res/menu/card_browser.xml Outdated Show resolved Hide resolved
@criticalAY
Copy link
Contributor

The unit test fail as the col is not open I think

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.

Cheers

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.

Tests are failing

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.

I believe:

Subject: [PATCH] feat: Toggle Bury

Issue 14163
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt	(revision 9fb827d72dcbb33cfc10761fbed02216a09f0181)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt	(date 1715982065094)
@@ -851,14 +851,14 @@
     }
 
     private fun setFlagTitles(menu: Menu) {
-        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName
-        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName
-        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName
-        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName
-        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName
-        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName
-        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName
-        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName
+        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName()
+        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName()
+        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName()
+        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName()
+        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName()
+        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName()
+        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName()
+        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName()
     }
 
     @SuppressLint("RestrictedApi")
Index: AnkiDroid/src/main/java/com/ichi2/anki/previewer/PreviewerFragment.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/previewer/PreviewerFragment.kt b/AnkiDroid/src/main/java/com/ichi2/anki/previewer/PreviewerFragment.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/previewer/PreviewerFragment.kt	(revision 9fb827d72dcbb33cfc10761fbed02216a09f0181)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/previewer/PreviewerFragment.kt	(date 1715982065097)
@@ -206,14 +206,14 @@
     }
 
     private fun setFlagTitles(menu: Menu) {
-        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName
-        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName
-        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName
-        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName
-        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName
-        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName
-        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName
-        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName
+        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName()
+        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName()
+        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName()
+        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName()
+        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName()
+        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName()
+        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName()
+        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName()
     }
 
     private fun setBackSideOnlyButtonIcon(menu: Menu, isBackSideOnly: Boolean) {
Index: AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt	(revision 9fb827d72dcbb33cfc10761fbed02216a09f0181)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/Flag.kt	(date 1715982602616)
@@ -18,19 +18,31 @@
 import androidx.annotation.ColorRes
 import androidx.annotation.DrawableRes
 import com.ichi2.anki.CollectionManager.TR
+import com.ichi2.anki.CollectionManager.withCol
 import com.ichi2.libanki.Card
 import com.ichi2.libanki.CardId
 import com.ichi2.libanki.Collection
 
-enum class Flag(val code: Int, @DrawableRes val drawableRes: Int, @ColorRes val browserColorRes: Int?, val displayName: String) {
-    NONE(0, R.drawable.ic_flag_transparent, null, TR.browsingNoFlag()),
-    RED(1, R.drawable.ic_flag_red, R.color.flag_red, TR.actionsFlagRed()),
-    ORANGE(2, R.drawable.ic_flag_orange, R.color.flag_orange, TR.actionsFlagOrange()),
-    GREEN(3, R.drawable.ic_flag_green, R.color.flag_green, TR.actionsFlagGreen()),
-    BLUE(4, R.drawable.ic_flag_blue, R.color.flag_blue, TR.actionsFlagBlue()),
-    PINK(5, R.drawable.ic_flag_pink, R.color.flag_pink, TR.actionsFlagPink()),
-    TURQUOISE(6, R.drawable.ic_flag_turquoise, R.color.flag_turquoise, TR.actionsFlagTurquoise()),
-    PURPLE(7, R.drawable.ic_flag_purple, R.color.flag_purple, TR.actionsFlagPurple());
+enum class Flag(val code: Int, @DrawableRes val drawableRes: Int, @ColorRes val browserColorRes: Int?) {
+    NONE(0, R.drawable.ic_flag_transparent, null),
+    RED(1, R.drawable.ic_flag_red, R.color.flag_red),
+    ORANGE(2, R.drawable.ic_flag_orange, R.color.flag_orange),
+    GREEN(3, R.drawable.ic_flag_green, R.color.flag_green),
+    BLUE(4, R.drawable.ic_flag_blue, R.color.flag_blue),
+    PINK(5, R.drawable.ic_flag_pink, R.color.flag_pink),
+    TURQUOISE(6, R.drawable.ic_flag_turquoise, R.color.flag_turquoise),
+    PURPLE(7, R.drawable.ic_flag_purple, R.color.flag_purple);
+
+    fun displayName() : String = when (this) {
+        NONE -> TR.browsingNoFlag()
+        RED -> TR.actionsFlagRed()
+        ORANGE -> TR.actionsFlagOrange()
+        GREEN -> TR.actionsFlagGreen()
+        BLUE -> TR.actionsFlagBlue()
+        PINK -> TR.actionsFlagPink()
+        TURQUOISE -> TR.actionsFlagTurquoise()
+        PURPLE -> TR.actionsFlagPurple()
+    }
 
     companion object {
         fun fromCode(code: Int): Flag {
Index: AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt	(revision 9fb827d72dcbb33cfc10761fbed02216a09f0181)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt	(date 1715982065104)
@@ -822,25 +822,25 @@
     }
 
     private fun setFlagTitles(menu: Menu) {
-        menu.findItem(R.id.action_select_flag_zero).title = Flag.NONE.displayName
-        menu.findItem(R.id.action_select_flag_one).title = Flag.RED.displayName
-        menu.findItem(R.id.action_select_flag_two).title = Flag.ORANGE.displayName
-        menu.findItem(R.id.action_select_flag_three).title = Flag.GREEN.displayName
-        menu.findItem(R.id.action_select_flag_four).title = Flag.BLUE.displayName
-        menu.findItem(R.id.action_select_flag_five).title = Flag.PINK.displayName
-        menu.findItem(R.id.action_select_flag_six).title = Flag.TURQUOISE.displayName
-        menu.findItem(R.id.action_select_flag_seven).title = Flag.PURPLE.displayName
+        menu.findItem(R.id.action_select_flag_zero).title = Flag.NONE.displayName()
+        menu.findItem(R.id.action_select_flag_one).title = Flag.RED.displayName()
+        menu.findItem(R.id.action_select_flag_two).title = Flag.ORANGE.displayName()
+        menu.findItem(R.id.action_select_flag_three).title = Flag.GREEN.displayName()
+        menu.findItem(R.id.action_select_flag_four).title = Flag.BLUE.displayName()
+        menu.findItem(R.id.action_select_flag_five).title = Flag.PINK.displayName()
+        menu.findItem(R.id.action_select_flag_six).title = Flag.TURQUOISE.displayName()
+        menu.findItem(R.id.action_select_flag_seven).title = Flag.PURPLE.displayName()
     }
 
     private fun setMultiSelectFlagTitles(menu: Menu) {
-        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName
-        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName
-        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName
-        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName
-        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName
-        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName
-        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName
-        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName
+        menu.findItem(R.id.action_flag_zero).title = Flag.NONE.displayName()
+        menu.findItem(R.id.action_flag_one).title = Flag.RED.displayName()
+        menu.findItem(R.id.action_flag_two).title = Flag.ORANGE.displayName()
+        menu.findItem(R.id.action_flag_three).title = Flag.GREEN.displayName()
+        menu.findItem(R.id.action_flag_four).title = Flag.BLUE.displayName()
+        menu.findItem(R.id.action_flag_five).title = Flag.PINK.displayName()
+        menu.findItem(R.id.action_flag_six).title = Flag.TURQUOISE.displayName()
+        menu.findItem(R.id.action_flag_seven).title = Flag.PURPLE.displayName()
     }
 
     private fun updatePreviewMenuItem() {

@criticalAY
Copy link
Contributor

CI is happy now

@david-allison david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author Needs Review labels May 17, 2024
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.

Cheers!

Copy link
Contributor

@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.

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 May 19, 2024
@lukstbit
Copy link
Contributor

Going in after #16427(with which I'm almost done)

@lukstbit lukstbit added this pull request to the merge queue May 19, 2024
Merged via the queue into ankidroid:main with commit ebbd10d May 19, 2024
8 checks passed
@github-actions github-actions bot added this to the 2.19 release milestone May 19, 2024
@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 May 19, 2024
Copy link
Contributor

Maintainers: Please Sync Translations to produce a commit with only the automated changes from this PR.

Read more about updating strings on the wiki,

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.

[BUG]: AnkiDroid to use Flag names like Anki
7 participants