sboesen / remotely-sync

fork of remotely-save with security upgrades
Apache License 2.0
212 stars 8 forks source link

[Feature Request]: Receive the notification 'Remotely Secure finish!' after 'Remotely Secure already running in stage...' #41

Closed MartinJDavis93 closed 11 months ago

MartinJDavis93 commented 1 year ago

What feature are you suggesting?

20231124184112

A rather debatable point, more aesthetic. Perhaps it's worth displaying only "Remotely Secure already running!" if synchronization is already in progress and you're trying to start it manually. Because various underscores somehow immediately associate with an error, and displaying the current synchronization stage is not fundamentally important.

What remote cloud services are you using, or suggesting adding the feature to?

No response

Ensure no sensitive information

sboesen commented 1 year ago

Just to check are you seeing this when you're not running a manual sync?

The plugin currently checks if the "triggerSource" is manual before displaying that alert, which from code review is only set when clicking the sync button in the sidebar or when running the command command_startsync.

Screenshot 2023-11-24 at 10 23 25 AM
MartinJDavis93 commented 1 year ago

Just to check are you seeing this when you're not running a manual sync?

The plugin currently checks if the "triggerSource" is manual before displaying that alert, which from code review is only set when clicking the sync button in the sidebar or when running the command command_startsync.

Screenshot 2023-11-24 at 10 23 25 AM

Yes, this is strictly for manual synchronization. I'm closing this issue since it's not important overall. The plugin works correctly. It's just that over the years, I've developed a habit of manually triggering synchronization every time I make changes to an existing note or create a new one. Considering that the time for automatic synchronization has now significantly reduced, throughout the day, I kept catching that notification, where the underline always triggers me as if there was some error 😅

sboesen commented 1 year ago

Thanks for clarifying, I reread and agree that the stage it's in might be too much info. I'll keep it as is for now but not strongly opposed to changing it if it continues yes being a bother.

MartinJDavis93 commented 1 year ago

Thanks for clarifying, I reread and agree that the stage it's in might be too much info. I'll keep it as is for now but not strongly opposed to changing it if it continues yes being a bother.

I have decided to raise this issue again as I have found the main reason that dissatisfied me in this regard. I felt that something was wrong, but I thought it was only about underscores in the long notification with the indication of the synchronization stage. The question is, why does the user initiate manual synchronization at all? It is to make sure that everything is synchronized accurately. And to find out if everything has been synchronized accurately, receive the notification - 'Remotely Secure finish!', which is currently missing. We receive the notification 'Remotely Secure already running in stage...', but nothing about completion. And we are not sure whether synchronization has already finished, or not, and whether we can close the application or still wait. And this is bad, especially for the mobile version, where the icon on the panel does not support animation, so it is unclear whether synchronization is taking place, so it is easier to start it manually. And since sometimes the file can be large, the time may be longer. Hence the problem of uncertainty about completion.

If during manual synchronization, when automatic synchronization was not started, we get:

1/2 Remotely Secure Sync Preparing (name of sync service) 2/2 Remotely Secure finish!

Then during manual synchronization, when it overlaps with automatic synchronization, it would be great to receive:

1/2 Remotely Secure already running in stage... (although, as I wrote, it can be shortened to a more visually pleasant: Remotely Secure already running!) 2/2 Remotely Secure finish!

So that the user knows for sure that synchronization is complete and everything has been saved.

P.S. Regarding 'Sync on Save' and the shortened time for automatic synchronization. Even in this short interval of slightly more than a day, I have already encountered several times that synchronization did not occur in automatic mode. But it's not a problem; it's just the time interval that is genuinely necessary for synchronization, because a certain waiting time is needed for the synchronization of changes. And so that you can make changes and close the application, most likely, you will not get into the synchronization window and the changes made will not be synchronized accordingly. Even if the time for automatic synchronization is reduced to 1 second (which, in my opinion, is already absurd), we still have to wait a few seconds before we can close the application. Therefore, the habit of manually starting synchronization will not go away. And the 'Sync on Save' feature and the shortened time for automatic synchronization are more pleasant additions, just in case if you forget to start synchronization manually, there is a better chance of getting at least partially synchronized changes.

So, it would be great if you could implement the synchronization completion notification that I described.

sboesen commented 1 year ago

This makes sense to me, I agree.

Implementation note for later / anyone interested in taking this one up:

I think we can add a boolean like isManualSync to the RemotelySavePlugin class storing if the current sync is manual or automatic. If it's manual, display that final sync notification. Then new behavior:

If current sync is automatic and user manually syncs, set bool to true. After current sync is complete, set isManualSync to false (regardless of trigger source).

kadisonm commented 1 year ago

Hmm, I feel as though this would only be relevant on mobile as with PC I've just got into the habit of checking the status bar to see if it synced or not.

I do agree that it is hard to tell with mobile, and I have had times where I have manually synced just to check. Now that I think back, a completion notification would be handy to know once it's done so I can exit the app.

As a whole it is pretty tedious manually syncing to check on mobile, so maybe there could be another solution to check as well.

kadisonm commented 1 year ago

Would it be possible to add the sync status to the right hand menu on mobile? This is where you find the word count and backlinks. I asked on the Obsidian discord, so I'll let you all know once I get a response.

This is what I mean: https://github.com/sboesen/remotely-secure/assets/134670047/29ccb7fe-e014-46a0-8ae7-bf2e5c66f63e

MartinJDavis93 commented 1 year ago

Hmm, I feel as though this would only be relevant on mobile as with PC I've just got into the habit of checking the status bar to see if it synced or not.

I do agree that it is hard to tell with mobile, and I have had times where I have manually synced just to check. Now that I think back, a completion notification would be handy to know once it's done so I can exit the app.

As a whole it is pretty tedious manually syncing to check on mobile, so maybe there could be another solution to check as well.

It's important for both the desktop and mobile versions because it's the correct functioning, and you can observe the synchronization as you wish. Personally, I've turned off the status bar notification so it doesn't blink for me. Everyone configures it as they find convenient. This request only pertains to notifications. If you initiated synchronization by clicking on it, logically, you need to be notified when it's completed. And it doesn't matter whether the automatic mode was active or not. If you don't initiate manual synchronization, you won't receive any notifications.

MartinJDavis93 commented 1 year ago

Would it be possible to add the sync status to the right hand menu on mobile? This is where you find the word count and backlinks. I asked on the Obsidian discord, so I'll let you all know once I get a response.

This is what I mean: https://github.com/sboesen/remotely-secure/assets/134670047/29ccb7fe-e014-46a0-8ae7-bf2e5c66f63e

I completely disagree here. Because it's absolutely impractical. Also, very few plugin developers add shortcuts to the right panel. Mostly, these are plugins for managing something or adding features like a calendar or tables. Mobile devices have a small screen size, so only one panel can be opened there at a time. Perhaps this can somehow be applied to a tablet, but open the right panel to check the synchronization status seems quite strange and not practical. I hope over time, Obsidian developers will add support for icon animation on mobile devices, then it will be possible to see whether synchronization is running or not based on that.

kadisonm commented 1 year ago

I hope over time, Obsidian developers will add support for icon animation on mobile devices, then it will be possible to see whether synchronization is running or not based on that.

Yes but as of current I'd rather open the side panel and see the sync status as text rather than click on manual sync in the command palette. If mobile support animations then I would prefer an animation as well.

Because it's absolutely impractical

If it is supported then it would definitely be more practical for the reasons I listed above. Since you wouldn't have to run the sync command, and you could see when it was last synced.

A simple remotely secure view would work since you only have to click it once. I tested with calendar and once I click onto the calendar view and restart the app, the view persists and I don't have to click onto it again. Allowing you to just swipe left to view the status then swipe right to close it. Easy.

I've turned off the status bar notification so it doesn't blink for me

I'm not entirely sure what you mean by notification? If you could please elaborate that would be great. My understanding of the status bar is just that is constantly displays text next to the word count of when you last sync and if it's syncing.

kadisonm commented 1 year ago

logically, you need to be notified when it's completed.

Also I never denied this, I think it would be great for mobile as I agree. However, I'm just saying I think a view in the right side bar would be more convenient, atleast for me. Sorry if there was a misunderstanding.

sboesen commented 1 year ago

@kadisonm I agree with @MartinJDavis93 that I don't think this would be very practical for mobile, but happy to discuss further in a separate feature request if you'd like! I wish there was a good way to display the status sync message without a popup on mobile.

Perhaps this can somehow be applied to a tablet, but open the right panel to check the synchronization status seems quite strange and not practical.

I think tablet would be the only use case where this could make sense, but let's move that to another issue if you want to discuss that further. This issue is focused on updates to the alert sync popup. Thanks!

MartinJDavis93 commented 1 year ago

I'm not entirely sure what you mean by notification? If you could please elaborate that would be great. My understanding of the status bar is just that is constantly displays text next to the word count of when you last sync and if it's syncing.

Yes, that's exactly what I meant.

MartinJDavis93 commented 1 year ago

logically, you need to be notified when it's completed.

Also I never denied this, I think it would be great for mobile as I agree. However, I'm just saying I think a view in the right side bar would be more convenient, atleast for me. Sorry if there was a misunderstanding.

Understand that the synchronization button is intended not for viewing the synchronization status but for initiating synchronization. Yes, the notifications signal exactly the start and end of synchronization, from which we learn about the status, more precisely, whether the synchronization has already been completed or not (because the status probably means whether the synchronization is currently running and, if so, at what stage it is now, and this is only when applying synchronizations, if started in manual mode). And now, if the syncs overlap, there is no completion notification. That is the whole point of my request. It's not known whether the synchronization has completed.

What you'll be able to understand from the label in the right panel: 'less than a minute ago'? Is it when the last sync was 1 second ago or 59 seconds? Unless, you've turned off the "Sync on Save" option, and at the same time, automatic synchronization is also turned off or set to more than 1 minute. Because otherwise you'll always have the inscription: "less than a minute ago" in the right panel. What is the use of this, I do not understand. Another option is that you've a very large storage, or a lot of notes are being sent for synchronization at the same time. But again, if you open the panel, you'll eventually want to synchronize the storage if it's not currently synchronized. And if you get the inscription: "less than a minute ago", it'll not be informative, unless, you didn't make any changes for the last minute. If it's been more than a minute, you'll most likely want to sync. So I don't understand the point of all these actions if it's simpler to manually start synchronization and be sure that everything is synchronized.

P.S. Yes, all this could be relevant if the synchronization status was in the variant: "synchronized 3 seconds ago, 5 seconds ago, etc.". Then it would make sense. But not in the current version.

kadisonm commented 1 year ago

Is it when the last sync was 1 second ago or 59 seconds?

I agree that this could be an issue. As of current I just assume that everything is fully up to date if it says less than a minute ago since I only sync my vault with sync on save.

But if you have sync on save off then I can see how that's an issue

MikeWi11s0n commented 12 months ago

Just came in to report this issue, but I see it is already been reported. I completely agree with the author. Yesterday, I installed the plugin, replacing Remotely Save, and noticed that it supports a shorter interval for autosync. That is awesome. Thank you. I have temporarily disabled Sync on Save. I have not fully figured out how it works yet. However, I still manually save every time I close Obsidian, or more precisely, manually sync. It is like in Word, there is automatic saving, but it is still better to save manually before closing for reliability. For me, it is the same here. I would greatly appreciate it if you could look into this issue as soon as possible.

MartinJDavis93 commented 12 months ago

Just came in to report this issue, but I see it is already been reported. I completely agree with the author. Yesterday, I installed the plugin, replacing Remotely Save, and noticed that it supports a shorter interval for autosync. That is awesome. Thank you. I have temporarily disabled Sync on Save. I have not fully figured out how it works yet. However, I still manually save every time I close Obsidian, or more precisely, manually sync. It is like in Word, there is automatic saving, but it is still better to save manually before closing for reliability. For me, it is the same here. I would greatly appreciate it if you could look into this issue as soon as possible.

Great to see that I'm not the only one here with such sync usage 😅

If it's not too much trouble, could you share your thoughts on whether it's worth shortening the message from '1/2 Remotely Secure already running in stage...' (for example, as in the screenshot above) to a visually simpler '1/2 Remotely Secure already running!'? Since you have a similar usage scenario to mine, I'd love to hear your opinion on this.

MikeWi11s0n commented 12 months ago

Great to see that I'm not the only one here with such sync usage 😅

If it's not too much trouble, could you share your thoughts on whether it's worth shortening the message from '1/2 Remotely Secure already running in stage...' (for example, as in the screenshot above) to a visually simpler '1/2 Remotely Secure already running!'? Since you have a similar usage scenario to mine, I'd love to hear your opinion on this.

🤣No problem at all. Well, yes, undoubtedly, without specifying the stage, it looks simpler and cleaner. In Remotely Save, there were a whopping 8 notifications displaying the synchronization stage, which I also didn't quite like. Here, they reduced it to two, already without specifying the stage, which, in my opinion, is much better. So, if it is possible to shorten the notification to a simple Remotely Secure already running!, I am all for it.

MartinJDavis93 commented 12 months ago

Great to see that I'm not the only one here with such sync usage 😅 If it's not too much trouble, could you share your thoughts on whether it's worth shortening the message from '1/2 Remotely Secure already running in stage...' (for example, as in the screenshot above) to a visually simpler '1/2 Remotely Secure already running!'? Since you have a similar usage scenario to mine, I'd love to hear your opinion on this.

🤣No problem at all. Well, yes, undoubtedly, without specifying the stage, it looks simpler and cleaner. In Remotely Save, there were a whopping 8 notifications displaying the synchronization stage, which I also didn't quite like. Here, they reduced it to two, already without specifying the stage, which, in my opinion, is much better. So, if it is possible to shorten the notification to a simple Remotely Secure already running!, I am all for it.

Great, thanks for your reply!

kadisonm commented 12 months ago

'1/2 Remotely Secure already running!'

I think so as well. Displaying the stage with this notification should probably go into debug mode.

sboesen commented 12 months ago

Implemented in 0.4.20.

sboesen commented 12 months ago

I think there is an edge case where the implementation might not work, but it's probably not likely to trigger. Please reopen if it's an issue.

Edge case specifically: 1) Automatic sync is triggered 2) Manual sync requested during automatic sync 3) A second automatic sync triggers (overwrites isManual to false), preventing the finished notification from displaying.

MartinJDavis93 commented 12 months ago

Implemented in 0.4.20.

Much obliged! So far, it's working well, no issues have arisen. If anything comes up, I'll be sure to let you know.

There's a small purely visual nuance. It would be great if you could add the text 1/2 before 'Remotely Secure already running!' as it indicates a stage which is also used for the second notification. Currently, we have: Remotely Secure already running! 2/2 Remotely Secure finish!

Thanks again!

sboesen commented 12 months ago

Reopening to track that request, I agree that makes sense. Happy to get it implemented!

MartinJDavis93 commented 12 months ago

Reopening to track that request, I agree that makes sense. Happy to get it implemented!

I don't know how relevant it is to re-open the issue, since it is simply necessary to add 1/2 as a text before Remotely Secure already running! Is it, it's in order not to forget 😄

MikeWi11s0n commented 12 months ago

Implemented in 0.4.20.

Thank you very much!

sboesen commented 12 months ago

Yeah I just don't want to forget :) and no problem. Should be a quick fix but have to edit the language files and code using it, no's like a 4 line code fix in two repos.

MartinJDavis93 commented 12 months ago

How important is it from a practical point of view? Practically, not important at all.

But as I've pointed out many times, it's just the right thing to do and yes, it should work. And it's these little nuances that contribute to the overall impression. The fix itself is straightforward and shouldn't take more than 10 minutes. So, it's better to have it than leave it as it's now.

Getting to the point, today, while in an elevator, I opened Obsidian on my phone to add changes to an already existing note. This note had not been synchronized before. As it was an elevator, the network was intermittent, and the connection was slow. In short, the note didn't appear, so I initiated manual synchronization and received 'Remotely Secure already running!' as a notification. I didn't receive a finish notification, even though the synchronization had actually occurred, as the file appeared. It turns out I started manual synchronization before the automatic synchronization at startup was complete. Since you've probably made changes only for notifications when synchronization overlaps only on automatic, it seems that when manual synchronization overlaps with startup, there is no completion notification.

Of course, this is, to put it mildly, not a very common case, and practically, it won't be a problem for anyone, as few will notice it. But, in my opinion, such nuances are crucial in the overall picture of how the development is approached. And once again, if there's a notification for the start, it's logical to have one for finish. If fixes of this nature can be implemented and don't take much time, it's better to make them. This will completely address the issue of notification functionality once and for all.

P.S. Of course, these are not urgent fixes, just pointing out this nuance for your awareness.

MartinJDavis93 commented 11 months ago

@sboesen When making changes for the next update, please fix these inconveniences regarding notifications, as we encounter them every time during synchronization. I would sincerely appreciate it.

sboesen commented 11 months ago

Yeah will try to get this closed in next update, but will release it if it's taking too much time. The notification numbering is much easier to update than making sure every combination of manual/automatic sync is notifying properly, not really sure why that is happening right now

MartinJDavis93 commented 11 months ago

Yeah will try to get this closed in next update, but will release it if it's taking too much time. The notification numbering is much easier to update than making sure every combination of manual/automatic sync is notifying properly, not really sure why that is happening right now

Got it. I'll wait until when you've enough free time for this correction, just don't forget about it. Last time, you handled it very quickly, so I assumed that in this case, it won't take much of your time either. If updating the notification numbering is easy, you can start by fixing just that, and the rest can be addressed when you've the opportunity. And I apologize for bothering you once again.

sboesen commented 11 months ago

It's not a bother at all - I'll do what I can for sure and appreciate the follow-up. The numbering is definitely easy and will try to get that into the next release. The reason the other one is hard is I can't understand why this would happen (based on the code) and I'm having issues reproducing the behavior myself. Will just need more time to deep dive

MartinJDavis93 commented 11 months ago

It's not a bother at all - I'll do what I can for sure and appreciate the follow-up. The numbering is definitely easy and will try to get that into the next release. The reason the other one is hard is I can't understand why this would happen (based on the code) and I'm having issues reproducing the behavior myself. Will just need more time to deep dive

Is this you regarding #59? Well, I personally haven't encountered this issue. As far as I know, no one in my circle has experienced it either, although there are people who use OneDrive for synchronization. Therefore, I can't say anything about it.

sboesen commented 11 months ago

so I initiated manual synchronization and received 'Remotely Secure already running!' as a notification. I didn't receive a finish notification, even though the synchronization had actually occurred, as the file appeared. It turns out I started manual synchronization before the automatic synchronization at startup was complete. Since you've probably made changes only for notifications when synchronization overlaps only on automatic, it seems that when manual synchronization overlaps with startup, there is no completion notification.

Naw, I was referring to this. Not sure how reproducible. Regardless will get the name change in - if you still experience the finish message not appearing I say we open that as a separate issue focused on that.

MartinJDavis93 commented 11 months ago

Everything is great! With the latest update, everything is working perfectly. All the inaccuracies I noticed before have been fixed. Thanks for that ❤️

sboesen commented 11 months ago

Glad to hear!!