Skip to content
New issue

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

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

Already on GitHub? # to your account

NF: Gesture refactor to allow for mappable gestures #9142

Merged

Conversation

david-allison
Copy link
Member

@david-allison david-allison commented Jun 24, 2021

Purpose / Description

#8078 contains a large refactor to our gesture processing which allows arbitrary mapping of key inputs to commands

Currently, we map from Gesture -> Command

And we want to map from Command -> [Gesture] after the linked PR is completed

This brings the code closer to this PR, in a series of non-functional commits

The aim is that this change is fairly trivially reviewable, so we get closer to the idea

EDIT: Ref: #6052 (link is out of order)

How Has This Been Tested?

Unit tested

Checklist

  • 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

This exists because a good PR was proposed, but was not split up

Add "isBound/isEnabled"

Also adds tests

Refactor towards 6052
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

LGTM - this will be an interesting feature for 2.16 (more general mapping, and support for more peripherals, as this all lands, yes)

@mikehardy mikehardy added the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jun 24, 2021
@mikehardy
Copy link
Member

This doesn't look right?

> Task :AnkiDroid:compilePlayDebugAndroidTestJavaWithJavac FAILED
/home/mike/work/AnkiDroid/Anki-Android/AnkiDroid/src/androidTest/java/com/ichi2/anki/reviewer/PeripheralKeymapTest.java:45: error: incompatible types: invalid method reference
        PeripheralKeymap peripheralKeymap = new PeripheralKeymap(MockReviewerUi.displayingAnswer(), processed::add);
                                                                                                    ^
    incompatible types: ViewerCommand cannot be converted to Integer
Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
1 error

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label Jun 24, 2021
@david-allison david-allison removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jun 24, 2021
We want to move from "gesture" -> "command"
to "command" -> [gestures]

to allow for arbitrary gesture mapping

Moving to an enum allows more flexibility as a command becomes
a first-class concern

Refactoring towards issue 6052
Using 8078 as a guide
Gets closer to fixing 6052
@david-allison david-allison force-pushed the 6052-further-gesture-refactor branch from 125850f to d825cab Compare June 25, 2021 00:08
@david-allison david-allison added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Jun 25, 2021
@david-allison
Copy link
Member Author

Gah, sorry. Missed the androidTest classes, they're not built on a rebuild

@mikehardy
Copy link
Member

All clean and passes this time ⚡

@mikehardy mikehardy merged commit 63f51f9 into ankidroid:master Jun 25, 2021
@david-allison david-allison deleted the 6052-further-gesture-refactor branch June 25, 2021 16:04
@mikehardy mikehardy added this to the 2.16 release milestone Aug 22, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants