Skip to content
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

Deck options: 'back' should only request confirmation if changes are made #14438

Closed
Arthur-Milchior opened this issue Sep 16, 2023 · 14 comments · Fixed by #15503 or #17890
Closed

Deck options: 'back' should only request confirmation if changes are made #14438

Arthur-Milchior opened this issue Sep 16, 2023 · 14 comments · Fixed by #15503 or #17890

Comments

@Arthur-Milchior
Copy link
Member

Running on HEAD. If you open the deck option and change them then press back they are discarded. Please instead ask whether they should be cancelled or saved.

(Also, the save button is quite small. I doubt it satisfies accessibility requirement)

@dae
Copy link
Contributor

dae commented Sep 16, 2023

The first point would require some work to implement, as currently the frontend doesn't have a flag that tracks when a change has been made. You'd need to store a copy of the initial data, then provide some JS method that compares the current state with the initial state to see if anything has changed. The Kotlin code would then need to invoke it and decide what to do based on its response.

@david-allison
Copy link
Member

david-allison commented Jan 2, 2024

User comment:

Every time we tap Save button, the Deck options screen closes and we need to go back again and again to Deck options to continue our job. I think the Save button is not necessary or at least it shouldn’t close the screen.

https://forums.ankiweb.net/t/improvements-on-deck-options-screen-when-the-new-backend-enabled/39101

@briankrznarich
Copy link

I was actually coming here to consider filing a similar ticket. The title would have been something like:
"Inconsistent back-button behavior between AnkiDroid core settings and Deck settings" .

For the internal app settings (hamburger menu->settings), "back" will unconditionally save, which I think is how Ankidroid has always worked, even for deck-specific settings. Now, as you know, deck settings require the user to tap "save", or your changes will be silently discarded.

I screwed this up 4 or 5 times before I realized why my settings changes were not sticking. It doesn't take much work to adapt to, but every ankidroid user is going to screw this up 4 or 5 times before they figure out what's going on. A confirmation warning would alleviate this problem.

A long time ago I disliked this "back = unconditional save" mechanic, with no way to easily undo accidental changes. But all of iOS works this way, and most of Android I think. People are losing an intuition for an explicit "Save" button on phones, especially a tiny button at the top-right of the screen.

I agree with the earlier remarks on button-size and accessibility. The "save" button, and especially the drop-down menu to the right of the "save" button, are very very small. Besides being made physically taller, padding would probably help. There is literally 0 padding between the right side of the "Save" button and the edge of the screen. Whitespace on the right would make a safer tap-target for the drop-down menu.

@david-allison
Copy link
Member

@briankrznarich Thank you for your time, I agree and I'll get this resolved

@david-allison
Copy link
Member

Keep this open until we ask for confirmation on change, rather than unconditionally

@github-actions github-actions bot modified the milestones: 2.18 release, 2.17 release Feb 15, 2024
@david-allison david-allison reopened this Feb 15, 2024
@david-allison david-allison changed the title In deck option, in case of change, back should request confirmation Deck options: back should only request confirmation if changes are made Feb 17, 2024
@david-allison david-allison changed the title Deck options: back should only request confirmation if changes are made Deck options: 'back' should only request confirmation if changes are made Feb 17, 2024
@mikehardy mikehardy modified the milestones: 2.18 release, 2.19 release May 13, 2024

This comment has been minimized.

@github-actions github-actions bot added the Stale label Aug 12, 2024
@user1823
Copy link
Contributor

Still looking for a solution...

@github-actions github-actions bot removed the Stale label Aug 12, 2024
@Arthur-Milchior
Copy link
Member Author

I just opened this PR ankitects/anki#3410 , if DAE accepts it, once it's included in AnkiDroid, we can query the javascript to know whether there was any change.

@mikehardy mikehardy modified the milestones: 2.19 release, 2.20 Release Oct 8, 2024
@user1823
Copy link
Contributor

user1823 commented Nov 6, 2024

The backend changes have been incorporated into the AnkiDroid main branch. So, an interested dev can work on this now.

Arthur-Milchior added a commit to Arthur-Milchior/Anki-Android that referenced this issue Nov 8, 2024
Arthur-Milchior added a commit to Arthur-Milchior/Anki-Android that referenced this issue Nov 9, 2024
Arthur-Milchior added a commit to Arthur-Milchior/Anki-Android that referenced this issue Nov 9, 2024
The test don't have to press "ok" anymore as there are no changes.
Sadly, it seems that the javascript is not executed, so the test halts

Fixes ankidroid#14438
Arthur-Milchior added a commit to Arthur-Milchior/Anki-Android that referenced this issue Nov 10, 2024
The test don't have to press "ok" anymore as there are no changes.
Sadly, it seems that the javascript is not executed, so the test halts

The tests are deleted. As they use real webview, they can't be saved
as-is. And for some reason LongPress don't work on integration test in
the deck picker, which means I can't recreate the same test starting
with opening the deck options.

Fixes ankidroid#14438
Arthur-Milchior added a commit to Arthur-Milchior/Anki-Android that referenced this issue Nov 11, 2024
The test don't have to press "ok" anymore as there are no changes.
Sadly, it seems that the javascript is not executed, so the test halts

The tests are deleted. As they use real webview, they can't be saved
as-is. And for some reason LongPress don't work on integration test in
the deck picker, which means I can't recreate the same test starting
with opening the deck options.

Fixes ankidroid#14438
@mikehardy mikehardy modified the milestones: 2.20 Release, 2.21 release Dec 4, 2024
@david-allison
Copy link
Member

@Arthur-Milchior
Copy link
Member Author

Yes but it'll be broken with anki backend change because I removed the Bridge command that I introduced before knowing it was deprecated

@david-allison
Copy link
Member

That's fine. We won't update the backend until they start on RCs

@user1823
Copy link
Contributor

To clarify, Arthur didn't remove the implementation from upstream. He replaced it with another one. Here's the PR: ankitects/anki#3571

So, I guess AnkiDroid would have to modify some of its own code after the backend update.

@david-allison
Copy link
Member

Things move fast! This is now actively in the pipeline:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants