-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat(new reviewer): implement some of the actions #16493
Conversation
136f1c9
to
e182f64
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I think it's fine to defer work but I'm afraid of losing track of the state of the work if there are things that look really really close to complete (as much of this does) but has tiny things like two menu items slightly off center Perhaps a comment on the main port PR (or a checkbox-list in the description?) where tiny things are tracked? |
The important TODOs are documented in the code. Keeping a separate list would be mostly busywork to me at the moment, since most of the things aren't implemented yet. Maybe when this comes closer to be under a public-available experimental setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dev-only change. I'm happy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
Left some minor nitpicks plus there were changes in the flag area recently and this doesn't build at the moment.
AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt
Outdated
Show resolved
Hide resolved
This needs me to update the flags to use the custom names. |
to remove some of the indent
It is technically possible (although unlikely) to get an ID conflicts when using the ordinals as IDs. Also, I think that `onMenuItemClick` is easier to follow this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, LGTM!
Purpose / Description
Implement in the new reviewer:
Fixes
Part of #14302
Approach
They are implemented in separate commits, so they can be reviewed individually and split if necessary
Note: the
Mark note
andFlag
icons are misaligned for some reason and will be fixed laterAdding
Undo
to the snackbar of some of the actions will also be done laterHow Has This Been Tested?
Emulator 34
rev2.webm
rev21.webm
flag_mark.webm
user.action.webm
undo.redo.webm
Checklist
Please, go through these checks before submitting the PR.
rev21.webm