Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard

Next/Previous card gestures in Previewer

Open alyoshenka opened this issue 3 years ago • 15 comments

Purpose / Description

This fix introduces gesture support for viewing the next and previous card in Previewer.

Fixes

Fixes #12390

Approach

This adds the relevant viewer commands and checks for them in Previewer when a gesture is detected.

From what I could see, there are no other commands that have confusingly similar meanings. To the best of my knowledge, these new commands are set up like the others, however I may have missed something.

As per the conversation in the original issue, there are more commands that could be added to the Previewer, such as flipping the card. This pr just fixes the main issue, but I am looking into others to add as well.

How Has This Been Tested?

  1. Go to Settings > Controls > Set 'Preview Next/Previous Card' to the desired gesture
  2. Go to Previewer
  3. Card Browser > Options Menu > Preview
  • [x] The selected gestures move to next/previous card
    • [x] 'Previous' gesture does nothing when at the beginning of the deck
    • [x] 'Next' gesture does nothing when at the end of the deck

Learning (optional, can help others)

I couldn't find documentation detailing how to add new gestures. It seemed pretty straightforward, but it would be worthwhile to add a documentation page for it.

Checklist

Please, go through these checks before submitting the PR.

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

alyoshenka avatar Nov 24 '22 22:11 alyoshenka

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,

github-actions[bot] avatar Nov 24 '22 22:11 github-actions[bot]

Hi. I think that the user wants to re-use the gestures they use on the reviewer instead of creating duplicates exclusive to the previewer.

So I believe that the goal here is to make some gestures like Show answer and Answer button X to work on the previewer as well.

As you are the issue creator, do you have any ideas @voidplayer ?

And oh, tests are failing

BrayanDSO avatar Nov 26 '22 09:11 BrayanDSO

What actions would showing the next and previous card map to? I couldn't find any that seemed appropriate.

alyoshenka avatar Nov 26 '22 20:11 alyoshenka

Yes, ideally same gestures work without having to define them

but go to next/previous card without showing answer is an action exclusive of the preview window and have no gestures

voidplayer avatar Nov 28 '22 12:11 voidplayer

Yes, ideally same gestures work without having to define them

but go to next/previous card without showing answer is an action exclusive of the preview window and have no gestures

For this issue specifically, I'm working on showing next/prev card. If I get around to supporting other gestures, I'll make a separate PR.

alyoshenka avatar Nov 29 '22 23:11 alyoshenka

After looking more at ViewerCommand, it does seem like this class is intended to be used only with Reviewer: all of the MapableBindings are connected to it. I feel like this means a larger redesign is necessary, but I'm not sure what that entails.

alyoshenka avatar Dec 01 '22 22:12 alyoshenka

My solution that seems to work is as follows:

  • keep all gesture actions in ViewerCommand.values()
  • add specific gestures for each AbstractFlashcardViewer (just Reviewer and Previewer in this case) to the companion object
    • so you would access previewer gestures with ViewerCommand.previewCommands
    • This has the downside of needing to repeat most commands into ViewerCommand.reviewerCommands, but the upside of making an easy place to define which commands go to which viewer
  • add a new PreferenceCategory to ControlsSettingsFragment and populate it with the Previewer commands

This comes with the downside that there would need to be a new translated string resource: "Previewer commands" (and potentially also changing "Command mapping" to "Reviewer commands'). However, this would be necessary anyway with the necessity of making the Previewer command category.

I'm working on getting this change ready to push, but it will take some time to get the string resources figured out. That's my solution, at least.

I have a WIP version here to showcase these changes

alyoshenka avatar Dec 04 '22 18:12 alyoshenka

There is also an issue where pressing a key in Previewer causes the screen to darken. This is an issue in main as well.

alyoshenka avatar Dec 04 '22 20:12 alyoshenka

This PR still needs translations. It is my understanding that now that I've added a string resource, it will go out for translation, and I don't have to do anything further. Also, I think that controls_main_category should be renamed to "Reviewer commands", but didn't want to mess with strings any more than I had to.

alyoshenka avatar Dec 04 '22 20:12 alyoshenka

Sweet! Didn't expect everything to be 100% the first time, glad to hear this approach is acceptable. I'll get those changes in as soon as I can.

alyoshenka avatar Dec 04 '22 22:12 alyoshenka

Not sure what's going on with ubuntu-latest tests, I ran them locally with no errors.

alyoshenka avatar Dec 07 '22 00:12 alyoshenka

Network flake; restarted.

dae avatar Dec 07 '22 00:12 dae

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

github-actions[bot] avatar Feb 21 '23 14:02 github-actions[bot]

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[bot] avatar Mar 08 '23 18:03 github-actions[bot]

I'll take this one to completion in 2.17 if there's no reply. Good feature to have.

david-allison avatar Mar 08 '23 18:03 david-allison