Page MenuHomePhabricator

[web] Handle clicking on notifications
ClosedPublic

Authored by michal on Feb 22 2023, 3:06 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 21, 10:07 PM
Unknown Object (File)
Fri, Mar 8, 9:34 PM
Unknown Object (File)
Tue, Mar 5, 12:47 PM
Unknown Object (File)
Feb 21 2024, 7:02 PM
Unknown Object (File)
Feb 21 2024, 1:55 PM
Unknown Object (File)
Feb 21 2024, 1:55 PM
Unknown Object (File)
Feb 21 2024, 1:55 PM
Unknown Object (File)
Feb 21 2024, 12:57 PM
Subscribers

Details

Summary

When user clicks a notification we want to:

  • if a comm tab is already focused navigate to a correct thread
  • if it's not focused we want to focus it
  • if all tabs are closed we want to open a new tab
Test Plan

Check if tab navigates to a thread, focuses if needed. Check that a tab is created if needen.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

web/push-notif/push-notifs-handler.js
116–117

Is the ? after serviceWorker necessary here? It seems like you check that on line 96

web/push-notif/push-notifs-handler.js
117

Shouldn't there be a return?

web/push-notif/push-notifs-handler.js
117

return; is implied at the end of a function

web/push-notif/push-notifs-handler.js
117

I meant that we have to return the cleanup lambda from the useEffect

return () => navigator.serviceWorker?.removeEventListener('message', callback);

is that not right?

@inka was right, I forgot a return for the cleanup callback. Also removed one ? but the second one is still needed because there's a function call after condition so flow can't be sure if the variable wasn't modified.

This revision is now accepted and ready to land.Feb 27 2023, 12:06 PM