-
Notifications
You must be signed in to change notification settings - Fork 94
✨ Confirm dialog when opening URLs #373
base: develop
Are you sure you want to change the base?
Conversation
c61e808
to
104a47a
Compare
@lifenautjoe I've marked this as ready for review. I haven't pushed the license update yet, but all the code should be here. One question: We talked about clearing the trusted domains list when logging out. This is currently not in since the user preferences in general aren't cleared on log out. Losing your list of trusted domains, and thus having to go through the "trust this domain" thing again for each, when logging out and then back in might be annoying. At the same time I can see why we don't want another person logging in to "inherit" the domain list. Clearing it on log out isn't difficult so I can add that if we want. Another, slightly more difficult, option would be to prefix the preference namespace with the user's ID (unless that is a security issue for some reason), so that the app remembers each user's settings separately. Of course, it does not feel very likely that multiple people keep logging in and out on the same phone, so maybe this is a non-issue in the first place? |
Generated localisation files and LICENSE-3RD-PARTY. For the license, the full license text is too long to include in the file itself (IMO). I left a note with a link to the license, but we can of course add the full license to the repo instead if we wish. Also, my previous comment on this PR still holds so don't forget to read that one. |
I like the idea of preventing clicking on a suspicious domain, but if i understand i right the preferred domains are stored local on the device? |
Yeah, it's all stored locally at the moment. There are two reasons for this:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed that when the popup to ask permission opens up,
If I select, "Never ask again", I cannot tap "Trust this domain" or "Cancel" but if I have selected trust this domain, I can tap never ask again but not cancel.
This should allow tapping anywhere at all times, so people should be able to switch between the two choices and even cancel if they change their minds, this added friction might be annoying.
I also think we should clear the trusted domains list on logout, since its an edge case that people will use multiple accounts on a single device anyway, so if the edge case happens we should err on the safe side.
I did the "disable Trust this domain when Never ask again is selected" thing to make it consistent with how the filter pages work (select the Connections circle -> all other circle checkboxes are selected and disabled). Can see how it can cause friction here, though. Will change.
Makes sense 👌 |
Also, the Cancel button will now always be enabled.
Nevermind, just saw that 371-posts-media adds more stuff to prefs. Don't necessarily want to clear all that so I'll just clear the url stuff for now. |
This change is made to match how 371-posts-media handles application settings localisation
I just updated this against develop. Doing so I noticed a couple of things that are missing and should be added before this is done:
|
This PR adds a confirmation dialog (in the form of a bottom sheet) that pops up when the user taps a URL, see screenshot below:
The user is given several options:
Remaining stuff to do:
www.images.google.com
->google.com
).❓ Maybe mark some domains as always trusted (e.g.Wait until the Okuna trusted proxies list is available.okuna.io
).