Page MenuHomePhabricator

[native] policy acknowledgment handler
ClosedPublic

Authored by kamil on Dec 6 2022, 7:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 9:00 PM
Unknown Object (File)
Sun, Dec 15, 8:59 PM
Unknown Object (File)
Sun, Dec 15, 8:51 PM
Unknown Object (File)
Fri, Dec 6, 10:30 AM
Subscribers

Details

Summary

Code responsible for showing modal with policy acceptance. Currently, only with Terms of Use & Privacy because it's the only police we have right now.

Test Plan
  1. Log in to app
  2. Change confirmed field for logged user in policy table
  3. Modal should appear (not immediately but after any request will be sent to keyserver)
  4. I accept should close modal and update value on keyserver db
  5. User should not be able to close modal using any method
  6. Check how app behave after showing modal, hiding app and opening app again

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Dec 8 2022, 6:55 AM
ashoat requested changes to this revision.Dec 8 2022, 10:42 AM
ashoat added inline comments.
native/root.react.js
243–245

We discussed having this rendered inside NavigationHandler. Does that not work? I forget, I seem to remember you may have created a Linear task or comment for this... can you link it here?

If putting it in NavigationHandler doesn't work, I'd love to have an explanation as to why

This revision now requires changes to proceed.Dec 8 2022, 10:42 AM
kamil requested review of this revision.Dec 9 2022, 4:13 AM
kamil added inline comments.
native/root.react.js
243–245

We discussed having this rendered inside NavigationHandler. Does that not work? I forget, I seem to remember you may have created a Linear task or comment for this... can you link it here?

If I remember correctly we were thinking about rendering it in NavigationHandler as it's the most suitable place but then we established that it won't work because to use this handler we need navigate() from useNavigation() and because of that it needs to be inside <NavigationContainer> (NavigationHandler is not inside NavigationContainer). Because it's connected to the entire root, not to one specific navigator nested in the root so it's the most appropriate place to render this. I didn't create an issue or comment because we were talking about this in our 1:1.

If putting it in NavigationHandler doesn't work, I'd love to have an explanation as to why

Handler needs to be in <NavigationContainer>. Theoretically, I think we could add action and then dispatch it instead of using navigate(), and we can render this in NavigationHandler but is it needed or is a better solution than the current one?

Not requesting changes because I'm about to go out, but please address the inline comment before landing

native/root.react.js
243–245

Can we move NavigationHandler inside NavigationContainer, and then move PolicyAcknowledgmentHandler inside NavigationHandler?

move handler into NavigationHandler

native/root.react.js
243–245

Addressed

Remove PersistGate before landing please

native/navigation/navigation-handler.react.js
47 ↗(On Diff #19329)

I don't think we need PersistGate here... the persistedStateLoaded check above handles it

This revision is now accepted and ready to land.Dec 13 2022, 4:17 PM
This revision was automatically updated to reflect the committed changes.