Conversation
leesio
force-pushed
the
safari-client-spike
branch
from
4de4d64 to
d9205d3
Compare
TomKemp
left a comment
TomKemp
left a comment
There was a problem hiding this comment.
The approach looks good. I've left a few comments for discussion.
| return RegistrationState.PERMISSION_PROMPT_REQUIRED; | ||
| } | ||
|
|
||
| async clearAllState() { |
There was a problem hiding this comment.
I'm not sure what the purpose of this method is for non-safari web notifications 🤔 I doubt you would want to show the permission request dialog box when clearing the state. I guess the behaviour of this method only makes sense if it assumes that the user has already given permission? And therefore it probably doesn't matter that this won't successfully ask for permission in Safari, because developers probably wouldn't want to do that. If that is the assumption then we should probably make that explicit with checks/errors. I can explain that if it didn't make sense.
| } | ||
|
|
||
| async start() { | ||
| await this._resolveSDKState(); |
There was a problem hiding this comment.
Maybe we should remove this call here, which detects subscription changes, so that the behaviour is consistent with Safari. Otherwise, developers might get bugs where the state SDK is different at different points in their code when running in Safari vs other browsers.
leesio
force-pushed
the
safari-client-spike
branch
from
d3a14d4 to
280ae74
Compare
leesio
force-pushed
the
safari-client-spike
branch
from
4cf14ef to
d672b11
Compare
Safari devices will include the web push ID
Also npm run format
Dummy method for requesting website push id
Split register device methods
Move shared config into base client
Add not implemented methods to abstract base class
The state management is a little bit different to on web-push because the device token should (almost) never change and we can't freely check the stored token before requesting permission (because of how strict the safari is at enforcing permission request from a user gesture). This means the start behaves a little differently: - if the permission is default, immediately request permission - register the device - update the internal state
get is close to a synonym of request, hopefully getCurrentPermission is clearer
Clear all state implementation
Fetch website push ID
leesio
force-pushed
the
safari-client-spike
branch
from
67ee6aa to
993766c
Compare
- always return self from start method - start method should fail if permission request throws - use this._platform when setting user id
Safari bug fixes
2 participants
PoC for Safari Web Notifications