Anki-Android
Anki-Android copied to clipboard
Update FAB UI for more intuitive double tapping
Purpose / Description
Updating FAB UI for more intuitive double tapping
Fixes
Fixes #11121
Approach
-
Removed all instances of
mAddNoteLayout
as we don't need that view anymore. We will handle that functionality inside the FAB. -
Create new variables for drawable images to show in FAB (one for showing
add
icon and one for showingadd note
icon -
Create animations for FAB when it is pressed to show Add Note Icon and changed it's background color. Also when FAB is closed it's icon will change back from
Add Note
toAdd
-
Use attributes to set the background color of FAB depending upon different themes
-
Create a new function to handle the FAB animations in case a user selects
Add Decks
orGet Shared Deck
-
Removes instances of
Add note layout
from theDeckPickerFloatingActionMenuTest
so unit tests don't fail
How Has This Been Tested?
Manually tested on my Physical Device
- Before
https://user-images.githubusercontent.com/40427461/187999753-8305ecad-b309-41ac-b93b-4cbcf0632bad.mp4
- After
https://user-images.githubusercontent.com/40427461/187999843-d8db89da-e046-409c-bb64-bb94ae1d6095.mp4
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
I suggest making the Fab only add new notes. Other actions can be moved to the menu or to the bottom of the deck list.
Alternatively, remove the fab entirely and put the Add note action into the navigation drawer. If we do that, we could also make it possible to leave the Add note activity without saving changes; you could leave it and come back and see your unsaved input restored.
I suggest making the Fab only add new notes. Other actions can be moved to the menu or to the bottom of the deck list.
Alternatively, remove the fab entirely and put the Add note action into the navigation drawer. If we do that, we could also make it possible to leave the Add note activity without saving changes; you could leave it and come back and see your unsaved input restored.
Sounds like a decent new change. Should make it as a new issue, so that everyone can have their views before someone even start to work on it.
Unit Tests are failing because of removal of Add Note Layout from DeckPickerFloatingActionMenu
This is how it looks currently :
https://user-images.githubusercontent.com/40427461/189486678-910613cb-8e40-4093-a937-3199a54365c5.mp4
Only saw the video / haven't looked at the code deeply.
- David's request about keeping the label wasn't addressed
- Tests fail
- Indigo doesn't look good to me and won't match Plain, Dark and Black themes. I suggest creating an attribute and set it on each.
themePrimaryDark
should be good for the day themes andthemePrimaryLight
for the night themes should be good. If any of them doesn't exist, you can create it or propose another
@BrayanDSO I am sorry but I didn't understood it "We need label for the action". Can you explain like what has to be done for this or what does it mean ?
@oyeraghib. See the "Get shared decks" and "Add deck" text the buttons' side? That's it. There was one for "Add" as well, but this PR is currently removing it
This is how it looks currently :
https://user-images.githubusercontent.com/40427461/190522960-a8f72a0b-ebdb-4393-b0b0-07a9107bc539.mp4
It is a little bugged on Plain
theme. A white halo/ripple persists after tapping the button. Tested on API 31 device and API 25 emulator
@BrayanDSO I checked it on my API 29 device and it doesn't seem to be there.
Can you try checking on the build without this PR. As far as I know, I did not introduced anything which might have caused something like (halo
) this. But still I would like to know if anyone else can also check this up and let me know.
My apologies for being late on this one. So finally fixed the halogen artifact being shown on Plain themes ( I found it while running the app on Android 13 physical device ).
Before :
https://user-images.githubusercontent.com/40427461/196524715-77954d8c-683b-4ae4-8489-7255d843282d.mp4
After :
https://user-images.githubusercontent.com/40427461/196523959-f7d34d8b-fb4f-4ded-a52e-be2bcf03af2f.mp4
The reason : That halogen icon was Android's material shadow artifact, which comes on translucent backgrounds ( therefore it was only visible on the Plain themes - because on pressing the FAB for plain themes we use a different color for background which has lower opacity.
One of the solutions for it on SO mentioned the use of libraries : https://stackoverflow.com/questions/70227950/how-can-we-fix-the-elevation-shadows-on-transparent-translucent-views-without-ch which I thought would not be a good decision considering we only need it inside FAB. So the most ideal solution which I found for it was to make the background color of FAB opaque (in plain themes - on pressing the FAB) which fixes the issue.
Also added couple of animation tweaks ( by updating the durations, translations etc) to make animation look more smoother.
Question: do we still need to have a DoubleTapListener
set up after this PR?
Question: do we still need to have a DoubleTapListener set up after this PR?
IMO it is nice to have DoubleTapListener
for the reason that it could handle the case if the user double click the FAB quickly ( before the animation is completed ).
It also separates the functionality of the double tapping FAB to open note from animation. So if in future someone wants to change animation for FAB ( work on it's duration - increase/decrease ) it won't have affect on the functionality to open the note. For someone who has habit of double tapping the FAB they won't have any issues because of animation duration.
One meta advice, @oyeraghib: you can update your initial PR message. That would be great so that it always has the most up to date video, so that a new reader, or someone who forgot what this PR contains, know in what state the issues currently is.
Regarding the potential action in the FAB: I really want to keep "shared deck" in it. Because the FAB is probably the most obvious things on a first openining of the application, when the user is trying anki* for the first time with us. And so, ensuring they get an easy way to find some shared deck, and try the app without having to create their own cards immediately seems very important to me. Plus, if we were to remove the FAB, we should change the message shown when the collection is empty
@Arthur-Milchior Thanks for the feedback. I have updated the PR message with updated before and after videos. Regarding the FAB, I agree with your view
The FAB is probably the most obvious things on a first opening of the application, when the user is trying anki* for the first time with us. And so, ensuring they get an easy way to find some shared deck, and try the app without having to create their own cards immediately seems very important to me.
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
Hello 👋, this PR has been opened for more than 1 month 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
needs rebasing onto main
& de-conflicting
needs rebasing onto
main
& de-conflicting
made the required changes.
Tested on API 21 emulator, looks great!
Implementer's choice: I don't like the instant change in color if the user has disabled animations and would rather it didn't occur
To me it makes bit of sense in a way - that it tells the user that functionality of that button has been changed. For users who have been using AnkiDroid for long time might press it again (thinking it's a cross symbol) to close it . A bit of change in color can be a reason of focus and make them see that pressing it again will open the note instead of closing it.
I missed the change in color. Thanks David for catching it. I'll agree with David here.
Some people, in particular on e-ink device, really care about the lack of animation. I believe that noting a change in behaviour does not justify the cost for them. After all, it's really not clear they'll notice the color started changing. And in the worst case, they may learn very quickly that the behavior changed
Updated with discussed changes.
After updating :
https://user-images.githubusercontent.com/40427461/222978128-655dd0e2-c194-4875-be8e-92cf96075eec.mp4
I have animations disabled in the Developer settings, this doesn't handle that case.
@david-allison do you have any idea if there already defined function in AnkiDroid that can be used to check device's animation status ? Or should I be writing a new one.
I am not very experienced with testing at the moment. Seems like tests are failing due to adding that function which returns the status of ( if device's animation ) is enabled or not. Any suggestions how can I use it inside the DeckPickerFloatingActionMenuTest.kt
to make tests pass.
I believe it is a preferences setting, settings -> advanced -> safe display mode
And then there is a key in preferences (safeDisplay I think?) you may check
I don't believe we have the functionality yet.
To enable on your device: https://www.androidpolice.com/how-to-speed-up-animations-on-your-android-phone/
I don't believe we have the functionality yet.
To enable on your device: https://www.androidpolice.com/how-to-speed-up-animations-on-your-android-phone/
In developer options we have 3 animations. "Window animation", "Transition animation" and "animation duration". @david-allison may I know which of the following you have disabled so you are seeing the color change?
All 3
EDIT: Tried again. Failed with the same error as in the unit tests:
2023-03-04 03:49:22.072 19988-19988 MessageQueue-JNI com.ichi2.anki.debug E android.provider.Settings$SettingNotFoundException: window_animation_scale
at android.provider.Settings$Global.getInt(Settings.java:15261)
at com.ichi2.anki.DeckPickerFloatingActionMenu.deviceAnimationEnabled(DeckPickerFloatingActionMenu.kt:69)
at com.ichi2.anki.DeckPickerFloatingActionMenu.showFloatingActionMenu(DeckPickerFloatingActionMenu.kt:88)
at com.ichi2.anki.DeckPickerFloatingActionMenu.access$showFloatingActionMenu(DeckPickerFloatingActionMenu.kt:33)
at com.ichi2.anki.DeckPickerFloatingActionMenu$1.onUnconfirmedSingleTap(DeckPickerFloatingActionMenu.kt:279)
at com.ichi2.anki.ui.DoubleTapListener$detector$2.onSingleTapUp(DoubleTapListener.kt:86)
All 3
EDIT: Tried again. Failed with the same error as in the unit tests:
2023-03-04 03:49:22.072 19988-19988 MessageQueue-JNI com.ichi2.anki.debug E android.provider.Settings$SettingNotFoundException: window_animation_scale at android.provider.Settings$Global.getInt(Settings.java:15261) at com.ichi2.anki.DeckPickerFloatingActionMenu.deviceAnimationEnabled(DeckPickerFloatingActionMenu.kt:69) at com.ichi2.anki.DeckPickerFloatingActionMenu.showFloatingActionMenu(DeckPickerFloatingActionMenu.kt:88) at com.ichi2.anki.DeckPickerFloatingActionMenu.access$showFloatingActionMenu(DeckPickerFloatingActionMenu.kt:33) at com.ichi2.anki.DeckPickerFloatingActionMenu$1.onUnconfirmedSingleTap(DeckPickerFloatingActionMenu.kt:279) at com.ichi2.anki.ui.DoubleTapListener$detector$2.onSingleTapUp(DoubleTapListener.kt:86)
@david-allison please try with latest commits if possible and let me know if it is having any issues