Added Option to choose the default answer(Again, Hard, Good, Easy) on timeout when the "automatic display answer" setting is used #8281

IAmJaishree · 2021-03-20T05:07:40Z

Purpose / Description

Currently when "automatic display answer" setting is used on timeout by default "Again" gets selected and there is no option to change this setting.
This PR provides functionality of choosing the default answer for "automatic display answer" scenario.
Now by default "Good" is selected instead of "Again" as the answer.

Fixes

Fixes #2609

Approach

Added an option to change the default settings in the Preferences Page(Screenshot attached).
Based on the selected preference on timeout automatically answering by clicking the button.

How Has This Been Tested?

This has been tested manually by me by going through the possible scenarios.

Checklist

Please, go through these checks before submitting the PR.

You have not changed whitespace unnecessarily (it makes diffs hard to read)
You have a descriptive commit message with a short title (first line, max 50 chars).
Your code follows the style of the project (e.g. never omit braces in if statements)
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

Screenshots:

On Clicking on the "Automatic Display Answer" a popup appears.

IAmJaishree · 2021-03-20T05:10:48Z

I have currently hard coded a string as I was not able to add translations for all the languages and I am not aware of how this is done in Ankidroid.
It would be helpful if I get to know how this can be achieved, I will push the changes of removing the hard coded value in this PR after knowing it.

codecov · 2021-03-20T05:17:58Z

Codecov Report

Merging #8281 (214de59) into master (7921fc2) will decrease coverage by 0.03%.
The diff coverage is 11.76%.

@@             Coverage Diff              @@
##             master    #8281      +/-   ##
============================================
- Coverage     36.95%   36.92%   -0.04%     
- Complexity     3783     3797      +14     
============================================
  Files           354      356       +2     
  Lines         35820    35971     +151     
  Branches       4736     4774      +38     
============================================
+ Hits          13239    13282      +43     
- Misses        21072    21171      +99     
- Partials       1509     1518       +9

Impacted Files	Coverage Δ	Complexity Δ
...n/java/com/ichi2/anki/AbstractFlashcardViewer.java	`38.63% <11.76%> (-0.63%)`	`166.00 <1.00> (ø)`
...c/main/java/com/ichi2/anki/dialogs/HelpDialog.java	`34.09% <0.00%> (-6.30%)`	`1.00% <0.00%> (ø%)`
...id/src/main/java/com/ichi2/utils/VersionUtils.java	`16.21% <0.00%> (-5.41%)`	`1.00% <0.00%> (ø%)`
...in/java/com/ichi2/themes/StyledProgressDialog.java	`30.76% <0.00%> (-4.02%)`	`2.00% <0.00%> (ø%)`
...d/src/main/java/com/ichi2/utils/ClipboardUtil.java	`0.00% <0.00%> (-2.78%)`	`0.00% <0.00%> (-1.00%)`
...id/src/main/java/com/ichi2/anki/FieldEditText.java	`36.69% <0.00%> (-2.76%)`	`15.00% <0.00%> (-1.00%)`
...in/java/com/ichi2/libanki/AnkiPackageExporter.java	`57.32% <0.00%> (-2.52%)`	`9.00% <0.00%> (ø%)`
...roid/src/main/java/com/ichi2/utils/JSONObject.java	`47.50% <0.00%> (-1.67%)`	`31.00% <0.00%> (ø%)`
AnkiDroid/src/main/java/com/ichi2/libanki/DB.java	`86.06% <0.00%> (-1.54%)`	`41.00% <0.00%> (-1.00%)`
...iDroid/src/main/java/com/ichi2/libanki/Models.java	`82.13% <0.00%> (-0.74%)`	`140.00% <0.00%> (+7.00%)`	⬇️
... and 24 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7921fc2...214de59. Read the comment docs.

mrudultora · 2021-03-20T07:52:04Z

@IAmJaishree Hi, it's looking great. We don't have to add translations for all the languages. We just have to add the strings in the default one. Rest all work is done by crowdin. It will show the string label red at the time of addition but it will not cause the build to fail ;-)

mrudultora

It would be better using int here, over a String.

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

IAmJaishree · 2021-03-20T08:13:45Z

he string label red at the time of addition but it will not

Got it, thanks for this, I tried adding it in the default file and got red text as errors so I Hardcoded it instead.
Will make these changes :).

david-allison

Code looks good! Some code style nitpicks, ad a few translation changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

AnkiDroid/src/main/res/values/11-arrays.xml

AnkiDroid/src/main/res/xml/preferences_reviewing.xml

IAmJaishree · 2021-03-22T06:45:24Z

@david-allison-1 I have made the changes, please review :)

david-allison · 2021-03-22T08:00:36Z

@david-allison-1 I have made the changes, please review :)

Thanks! I'm trying a new workflow to ensure I'm not missing things. Could you add a row here (sheet: David - Blockers) for any tasks that I haven't responded to within a reasonable amount of time: https://docs.google.com/spreadsheets/d/1desofSHMBVWEhQUnDeFxcEefoBVT9fStfC6H_slUl0o/edit#gid=1274420398

david-allison

Looks good! One change, and one nice to have, then good to go. Thanks!

AnkiDroid/src/main/res/values/constants.xml

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

david-allison

Another request: could you ensure that the first line of the commit message is 50 characters or less

Leave a blank line after the title, and the longer commit description can be as long as you'd like, typically including the issue that is fixed, and 80 characters per line.

david-allison · 2021-03-22T14:19:05Z

Just a lint error to fix. We have duplicate strings, which adds unnecessary work for our translators.

Either add a comment="" attribute to the strings, or use the existing strings in the translation files

In this case Again can map to the same string that we use in the Reviewer UI (ease_button_again).

gradlew lintRelease on your local machine to confirm that the warnings have been resolved, then we're good to go.

  /home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/res/values/01-core.xml:70: Error: Duplicate string value "Again", used in automatic_answer_option_1, ease_button_again [DuplicateCrowdInStrings]
      <string name="ease_button_again">Again</string>
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      /home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/res/values/11-arrays.xml:98: Duplicates value in ease_button_again. Add a comment attribute to explain this duplication
```

This commit provides functionality of choosing the default answer for automatic display answer" scenario and by default "Good" is selected instead of "Again" as the answer. Fixes ankidroid#2609

david-allison

Looks good to me, thanks!!

mikehardy

🤔 but on the related issue, it was noted that in the other parts of the ecosystem (AnkiMobile specifically) "Bury" is the default action on the card:

#2609 (comment)

And for what it's worth, AnkiMobile's auto-advance default is to bury cards (least harm done if the user gets distracted), with options to use again/hard&good.

...so I was expecting bury to be the default option here, and within the set of available options for the preference.

Separately, in the Java code at least constants should be used vs hard-coded magic numbers 1, 2, 3 etc - not possible in the XML I think but in Java we have the 4 possible buttons at least

Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Consts.java

Lines 140 to 143 in d42a9e3

    
           public static final int BUTTON_ONE = 1; 
        
           public static final int BUTTON_TWO = 2; 
        
           public static final int BUTTON_THREE = 3; 
        
           public static final int BUTTON_FOUR = 4;

and I suppose the best we can do for bury is

Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Consts.java

Line 45 in d42a9e3

public static final int QUEUE_TYPE_MANUALLY_BURIED = -3;

(which will resolve to -3 and thus not collide with the button values)

I think the actual handling of the buttons and the code itself is pretty clean, just that bury isn't available and is not the default as I was expecting based on the connected issue

IAmJaishree · 2021-03-30T06:50:59Z

🤔 but on the related issue, it was noted that in the other parts of the ecosystem (AnkiMobile specifically) "Bury" is the default action on the card:

#2609 (comment)

And for what it's worth, AnkiMobile's auto-advance default is to bury cards (least harm done if the user gets distracted), with options to use again/hard&good.

...so I was expecting bury to be the default option here, and within the set of available options for the preference.

Separately, in the Java code at least constants should be used vs hard-coded magic numbers 1, 2, 3 etc - not possible in the XML I think but in Java we have the 4 possible buttons at least

Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Consts.java

Lines 140 to 143 in d42a9e3

public static final int BUTTON_ONE = 1;

public static final int BUTTON_TWO = 2;

public static final int BUTTON_THREE = 3;

public static final int BUTTON_FOUR = 4;

and I suppose the best we can do for bury is

Anki-Android/AnkiDroid/src/main/java/com/ichi2/libanki/Consts.java

Line 45 in d42a9e3

public static final int QUEUE_TYPE_MANUALLY_BURIED = -3;

(which will resolve to -3 and thus not collide with the button values)
I think the actual handling of the buttons and the code itself is pretty clean, just that bury isn't available and is not the default as I was expecting based on the connected issue

Should we make "Again" the default option?

mikehardy · 2021-03-30T13:35:59Z

@IAmJaishree I feel as though you did not read my comment completely.

Should we make "Again" the default option?

No.

My comment was specific and included a reference

on the related issue, it was noted that in the other parts of the ecosystem (AnkiMobile specifically) "Bury" is the default action on the card:

#2609 (comment)

github-actions · 2021-06-03T15:49:57Z

Hello 👋, this PR has been opened for more than 2 months 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

mikehardy · 2021-06-04T21:36:31Z

This is a very popular request - keeping open, will harvest the code for a fresh PR to get it done if it sits much longer

github-actions · 2021-08-03T21:38:02Z

Hello 👋, this PR has been opened for more than 2 months 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

mikehardy · 2021-08-05T02:54:50Z

okay - definitely needs harvesting, this is really popular. Up for grabs if anyone has time 🙏

github-actions · 2021-10-04T03:37:50Z

Hello 👋, this PR has been opened for more than 2 months 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

mikehardy · 2021-10-06T20:01:22Z

Never stale, I would love to see this in!

mrudultora reviewed Mar 20, 2021

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

david-allison requested changes Mar 20, 2021

View reviewed changes

IAmJaishree force-pushed the issue2609 branch 2 times, most recently from 19bc23f to 5dc0ec7 Compare March 22, 2021 06:42

david-allison reviewed Mar 22, 2021

View reviewed changes

AnkiDroid/src/main/res/values/constants.xml Outdated Show resolved Hide resolved

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Show resolved Hide resolved

david-allison requested changes Mar 22, 2021

View reviewed changes

IAmJaishree force-pushed the issue2609 branch from 5dc0ec7 to e37b462 Compare March 22, 2021 09:53

Added option to change default automatic answer

214de59

This commit provides functionality of choosing the default answer for automatic display answer" scenario and by default "Good" is selected instead of "Again" as the answer. Fixes ankidroid#2609

IAmJaishree force-pushed the issue2609 branch from e37b462 to 214de59 Compare March 23, 2021 04:45

david-allison approved these changes Mar 24, 2021

View reviewed changes

david-allison added the Needs Second Approval Has one approval, one more approval to merge label Mar 24, 2021

mikehardy added this to the 2.15 release milestone Mar 24, 2021

mikehardy requested changes Mar 24, 2021

View reviewed changes

mikehardy added Needs Author Reply Waiting for a reply from the original author and removed Needs Second Approval Has one approval, one more approval to merge labels Mar 24, 2021

mikehardy modified the milestones: 2.15 release, 2.16 release Apr 4, 2021

mikehardy mentioned this pull request May 24, 2021

Release 2.15 Speed Focus Mode on ankidroid #8921

Closed

github-actions bot added the Stale label Jun 3, 2021

mikehardy removed the Stale label Jun 4, 2021

mikehardy mentioned this pull request Jun 20, 2021

Feature request: Make reviewing cards a default action/ auto review #9102

Closed

github-actions bot added the Stale label Aug 3, 2021

mikehardy added Help Wanted Requesting Pull Requests from volunteers and removed Stale labels Aug 5, 2021

mikehardy mentioned this pull request Sep 8, 2021

[Feature request] Default answer selection when using the timers #9504

Closed

github-actions bot added the Stale label Oct 4, 2021

david-allison added the Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it. label Oct 4, 2021

mikehardy added Keep Open avoids the stale bot and removed Stale labels Oct 6, 2021

david-allison self-assigned this Oct 6, 2021

david-allison closed this Oct 14, 2021

david-allison removed Needs Author Reply Waiting for a reply from the original author Help Wanted Requesting Pull Requests from volunteers Keep Open avoids the stale bot Needs a new dev For PR that were good start but the original dev' left. Any dev can take over and finish it. labels Oct 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Option to choose the default answer(Again, Hard, Good, Easy) on timeout when the "automatic display answer" setting is used #8281

Added Option to choose the default answer(Again, Hard, Good, Easy) on timeout when the "automatic display answer" setting is used #8281

IAmJaishree commented Mar 20, 2021

IAmJaishree commented Mar 20, 2021

codecov bot commented Mar 20, 2021 •

edited

Loading

mrudultora commented Mar 20, 2021

mrudultora left a comment

IAmJaishree commented Mar 20, 2021

david-allison left a comment

IAmJaishree commented Mar 22, 2021

david-allison commented Mar 22, 2021

david-allison left a comment

david-allison left a comment

david-allison commented Mar 22, 2021 •

edited

Loading

david-allison left a comment

mikehardy left a comment

IAmJaishree commented Mar 30, 2021

mikehardy commented Mar 30, 2021

github-actions bot commented Jun 3, 2021

mikehardy commented Jun 4, 2021

github-actions bot commented Aug 3, 2021

mikehardy commented Aug 5, 2021

github-actions bot commented Oct 4, 2021

mikehardy commented Oct 6, 2021

	public static final int BUTTON_ONE = 1;
	public static final int BUTTON_TWO = 2;
	public static final int BUTTON_THREE = 3;
	public static final int BUTTON_FOUR = 4;

Added Option to choose the default answer(Again, Hard, Good, Easy) on timeout when the "automatic display answer" setting is used #8281

Added Option to choose the default answer(Again, Hard, Good, Easy) on timeout when the "automatic display answer" setting is used #8281

Conversation

IAmJaishree commented Mar 20, 2021

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

IAmJaishree commented Mar 20, 2021

codecov bot commented Mar 20, 2021 • edited Loading

Codecov Report

mrudultora commented Mar 20, 2021

mrudultora left a comment

Choose a reason for hiding this comment

IAmJaishree commented Mar 20, 2021

david-allison left a comment

Choose a reason for hiding this comment

IAmJaishree commented Mar 22, 2021

david-allison commented Mar 22, 2021

david-allison left a comment

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

david-allison commented Mar 22, 2021 • edited Loading

david-allison left a comment

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

IAmJaishree commented Mar 30, 2021

mikehardy commented Mar 30, 2021

github-actions bot commented Jun 3, 2021

mikehardy commented Jun 4, 2021

github-actions bot commented Aug 3, 2021

mikehardy commented Aug 5, 2021

github-actions bot commented Oct 4, 2021

mikehardy commented Oct 6, 2021

codecov bot commented Mar 20, 2021 •

edited

Loading

david-allison commented Mar 22, 2021 •

edited

Loading