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

Widget/shortcut should allow to sync #17084

Open
Arthur-Milchior opened this issue Sep 17, 2024 · 7 comments · May be fixed by #17399
Open

Widget/shortcut should allow to sync #17084

Arthur-Milchior opened this issue Sep 17, 2024 · 7 comments · May be fixed by #17399

Comments

@Arthur-Milchior
Copy link
Member

Let's say I've a single deck. I create a shortcut to the deck, or a widget that open the deck directly.
Then I never got an invitation to sync. I don't get an icon to invite to sync, nor a red or yellow label telling me syncing is needed, nor a regular reminder that no sync had occurred in quite some time. I assume all of you know this is how our users lose their data.

I think that the "sync" should be decoupled from the deck-picker so it can be reused elsewhere.
Then, when the reviewer is directly opened, when the deck picker is not on the stack, the sync option should appear in the reviewer.
Still, when the deck picker was used to open the reviewer, the sync button should not be present.

I guess the easiest way to do this is to pass a boolean in the bundle that indicates whether sync should be presented.

Contributors, note that this is only a suggestion. I want to get feedback from other maintainers without anyone start any implementation. If other maintainers agree this seems like a good idea, then we can start working on it

@Arthur-Milchior
Copy link
Member Author

@GuiSousa135 and @lutzky, may I ask what mean your thumbs up?
Is this also an isuse you had while using it? If so, you'll be happy ish to know I just created a PR for it.
If you meant you want to work on it, sorry, too late (also, I don't get notification for emojli so I would not have seen it)

@lutzky
Copy link

lutzky commented Nov 9, 2024

I meant the issue affects me too, thanks!

@Danika-Dakika
Copy link

Arthur -- seems like you're just about done with this feature, but it just occurred to me how close this is to one of my pet-peeves: there's no auto-sync when you enter the app directly into the card browser. (I'm talking specifically about the long-press "Card browser" option -- is that in the widget/shortcut category?) Is there any chance your fix would encompass that as well?

[As far as I can tell, there's no way to sync from the card browser at all -- but that might be a separate feature I need to finally get around to requesting ...]

@david-allison
Copy link
Member

david-allison commented Dec 12, 2024

Summarizing my point on Discord:

I don't support a menu item to sync in the Reviewer: each concept in the app should have one primary screen where it's managed, and Sync lives in the DeckPicker

We already have too many options in the Reviewer, and this further adds to the clutter

I fully support the ability for all screens to handle a sync occurring

I'd fully support a secondary action on screens to sync, where sensible (for example, in the nav, or a keyboard shortcut)

I'd fully support the widget starting a sync on click, in IntentHandler or similar (especially if auto sync is enabled). I think this handles @Danika-Dakika 's point

I fully support secondary actions on the screen to enable a sync (JS API/Gestures)

@Arthur-Milchior
Copy link
Member Author

I don't support a menu item to sync in the Reviewer: each concept in the app should have one primary screen where it's managed, and Sync lives in the DeckPicker

I admit I don't understand this "should". That seems far from self-evident for me.

We already have too many options in the Reviewer, and this further adds to the clutter

I agree there are many options, and so something should be added only if it really helps. I'm arguing in this case there is a case where it is the case.

I fully support the ability for all screens to handle a sync occurring

I'm not sure whether you mean:

  1. While syncing, we should be able to visit any screen
  2. It'd be acceptable if we could start syncing from all screens

Currently, on desktop, the collection sync block all UI. And reciprocally, there are a lot of UI that blocks the main window, such as opening the note type list/editor.

So I don't expect that we should allow screen navigation during the collection sync, so I am against 1.

And I'm also against 2 because a sync while we are editing a template, a note type, or a deck options either would ignore the change we are currently making (which would be confusing), or would be syncing a change not entirely finished, causing potentially a lot of bug on other devices when they download the partially edited note type.

I'd even argue we should note be able to sync when we are editing a note. Even if this can be done on anki. The reason is that, in anki, all changes to the notes are saved to the collection immediately. There is no notion of saving or discarding the changes. On the other paw, on ankidroid, any change to an existing note can be cancelled by closing the view without saving. So, the same question should then be asked, what's the meaning of a sync while a note is being edited, and the edition is not done?

I'd fully support a secondary action on screens to sync, where sensible (for example, in the nav, or a keyboard shortcut)

I'd fully support the widget starting a sync on click, in IntentHandler or similar (especially if auto sync is enabled). I think this handles @Danika-Dakika 's point

If the sync fails, we will need a dialog. I admit that I doubt it would be a great experience to show the dialog on top of the intent handler, given that it would be a dialog on top of an empty screen.
Also, that means that we would have no way to indicate that we are doing a background sync for the media. I believe that it's important to show it while it occurs so that users that are downloading a big decks with a lot of media at least have an hint about why some media are not displayed while they review

I fully support secondary actions on the screen to enable a sync (JS API/Gestures)

Appart from the reviewer, do you expect we could use a JS API anywhere?
I admit I can't even figure out when I'd want the note type or note to be in charge of telling to sync.

Currently, it seems to me that all gestures corresponds to action that the user can do through the menu (or navigation bar). I expect the "App bar buttons" and the "gesture" list to offer exactly the same choice. I used the "app bar buttons" to see all options available to me and decide what I want to have easily accessible.

I also admit I don't know what it would mean in terms of accessibility, given that gestures are not necessarily accessible. But I admit I don't have enough experience with this domain to know that my intuition is correct.

@david-allison
Copy link
Member

I agree there are many options, and so something should be added only if it really helps. I'm arguing in this case there is a case where it is the case.

I want to keep the options lean. We already have too many, especially given that the context of 'sync' is an app-wide operation, rather than a card-specific operation. I'll defer to @BrayanDSO here

  • A user can enable 'Automatic synchronization', and the Reviewer should sync
  • A user can sync using our intent-based API
  • A user should be able to sync via the JS API
  • A user can use a gesture to "Close reviewer and sync"

This seems sufficient to resolve the issue, without adding an additional menu item.

Note: given the following UI, it's less of a concern to add an option, but I still feel it adds to confusion.

Appart from the reviewer, do you expect we could use a JS API anywhere?

Yes, (note editor: custom importers, [image occlusion could have been an addon]), but off-topic


With apologies, do let me know if I missed replying to any points which were on-topic to unblocking the PR, rather than general discussions around syncing. I want to see things improve here.

@BrayanDSO
Copy link
Member

A user can enable 'Automatic synchronization', and the Reviewer should sync

IMO, this is by far the best option. If the auto-sync conditions are met, sync when the reviewer is opened and when the user leaves the app from the reviewer, same way the deck picker does. Eventually, this could be migrated to other or to all the screens. SyncWorker already can do that in the background when leaving the app, without any interruptions to the user.

That would also lead to removing Close reviewer and sync in #17586, which makes things simpler as well

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

Successfully merging a pull request may close this issue.

5 participants