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

Misleading or incomplete error message when attempting to enable push notifications while operating as linked device #414

Open
1 task done
Redjard opened this issue Dec 17, 2024 · 3 comments
Labels
bug Probable bug

Comments

@Redjard
Copy link

Redjard commented Dec 17, 2024

Guidelines

  • Yes, I have searched for existing issues

Bug Description

As addressed in #404, Molly doesn't support UP when operating as a linked device.
When trying to check "New activity while locked", this is communicated as "Sorry, this feature requires push notifications delivered via FCM or UnifiedPush, which are currently unavailable.".

When I came across this message, I interpreted it as Molly not identifying a UP provider on my phone, leading me down a useless path of identifying the issue before I stumbled over the correct answer, which is that Molly doesn't support it internally in my mode of usage.

If the warning were changed to something along "Sorry, no supported push notification provider (FCM or UnifiedPush) found or push notifications not available (e.g. operating as linked device)." that would be a lot less confusing.

Alternatively the error could be split out into two messages, one for notifications being unavailable on Signals side, and one for them being unsupported on the local device due to missing apps etc..

Screenshots

image

Device Model

Xperia 1 III

Android Version

Android 13, LineageOS 20

App Version

fdroid v7.26.1-1

Link to Debug Log

No response

@valldrac
Copy link
Member

Do you think it would be clearer to show an error message instead of hiding the unifiedpush option in a linked device?

@Redjard
Copy link
Author

Redjard commented Dec 18, 2024

Yes, I think so.
Imagine if there were other available methods, so you could actually enable the switch in question. You'd still want to inform the user why unified push wasn't available.

Popping the warning when selecting a specific method displays to the user a list of methods which are in theory supported, i.e. informs the user they are indeed in the correct place to enable them. And on the dev side it ensures an error message specific to the method.
In this case that specific message would ofc still need to differentiate specifically why UP isn't selectable.

Then the switch could maybe be reworked as "The selected method doesn't support push notifications due to xyz".

I'm actually still not clear on the full behavior here, I only used Molly as a linked device for now. Can websocket do locked notifications on primary devices?

If the "new activity while locked" toggle is actually a "modification" of the push notification type, then I think it makes sense to place it below the notification method, not into the "Notify when" category.
Rename it "Receive notifications while app is locked" (or maybe "not running", I don't have locking set up and I think that makes the wording somewhat confusing)

I tried to rework the UI:
image

Would look like this:
image

The "Push notifications delivery service" dropdown shows all methods Molly could in theory support. Selecting an unavailable method explains why the method in general is not available in the current configuration. Something like "UnifiedPush is unavailable when Molly is operating as a linked device."

When a valid method is selected, but doesn't support background notifications, a specific error message will pop up on tap of the "show notifications while closed" toggle. That error mentions the delivery method by name, and is something like "WebSocket doesn't support running in background when Molly is operating as a linked device."

I don't think the dropdown approach is ideal. I think all options should look identical, so the user is informed they can select them and will see something happen (i.e. the warning). But then the user won't know which of the options work until they try them all.
But if unavailable option were greyed out or unselectable the user probably wouldn't try to select them and see the reason as to why.

Maybe the selection option could be disabled and the reason shown directly inline below the option? The list is likely gonna be small enough, and even if there somehow are 10 unavailable options a scrollable selection dropdown seems like the smallest evil here.

image

The same could also work for background notifications, where somewhere on the "Show notifications while closed" toggle a warning would appear with the toggle being disabled, when the method doesn't support background notifications. That would potentially even allow saving the last state of the setting until a method is selected that does support background notifications.

@Redjard
Copy link
Author

Redjard commented Dec 19, 2024

I tested Molly as a primary device now and I see that I misunderstood the text I removed in my mockup.
Molly overrides which Delivery Service it uses and reverts to WebSocket temporarily if issues are detected.

In that case the text should remain. I would prefer a toogle for this behavior in its place, but that is for another issue/merge request.

@valldrac valldrac added the bug Probable bug label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Probable bug
Development

No branches or pull requests

2 participants