-
-
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: start Reviewer port #16455
feat: start Reviewer port #16455
Conversation
I haven't chosen the dark mode colors for the answer buttons yet, and choosing colors take time. Open to suggestions, but definitely don't want to bikeshed the PR because of colors. That always can be changed later |
UI: (Ik its in development mode but still pointing it out just in case) The answer button are bit overflowing ig ? |
This comment was marked as resolved.
This comment was marked as resolved.
This is the updated one in the PR or you plan to have it this way, as it has perfect alignment |
didn't copy that part when branching. Updated it. Thanks! (note: haven't updated the video ) |
currently, it does nothing. This commit is just to simplify the reviewing process
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.
LGTM, one question
- Note: [Bug]
OnRenderProcessGone
is not handled by Anki Desktop WebViews #15695 needs to be prioritized
AnkiDroid/src/main/java/com/ichi2/anki/ui/windows/reviewer/ReviewerViewModel.kt
Outdated
Show resolved
Hide resolved
the style and features are going to be implemented incrementally
ReviewerFragmentTest is basically a copy of ReviewerTest
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.
LGTM, with the understanding that this is development only
No point in blocking
Only encouragement is to add more tests - now is a great opportunity to improve coverage
@@ -0,0 +1,184 @@ | |||
/* | |||
* This program is free software; you can redistribute it and/or modify it under |
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.
No copyright?
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.
It's basically a copy of ReviewerTest, which doesn't have an author in the copyright header, and I didn't want to license it all under myself
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Also LGTM. UI design issues can be discussed later. |
Purpose / Description
I was going to finish it locally before pushing to avoid churn if I decide to refactor something, but I ended up procrastinating because of it. So, done is better than perfect.
It is currently a developer option. The design features are going to be implemented incrementally to ease the reviewing process and discussion.
When enough features are implemented, my plan is to move it into
Settings > Advanced > Experimental
Right now it rate cards and exits when finished. Awesome, isn't it?
Fixes
Approach
In the commits
How Has This Been Tested?
Emulator 34:
Light theme screenshot
reviewer.webm
Learning (optional, can help others)
Describe the research stage
Links to blog posts, patterns, libraries or addons used to solve this problem
Checklist
Please, go through these checks before submitting the PR.