Details

Reviewers

tomek
atul
ashoat

Commits

rCOMMf5d4c7c052b7: [native] policy acknowledgment handler

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

Log in to app
Change confirmed field for logged user in policy table
Modal should appear (not immediately but after any request will be sent to keyserver)
I accept should close modal and update value on keyserver db
User should not be able to close modal using any method
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 created this revision.Dec 6 2022, 7:18 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptDec 6 2022, 7:18 AM

kamil added a parent revision: D5831: [native] Terms of Use & Privacy modal.Dec 6 2022, 7:19 AM

Harbormaster completed remote builds in B14014: Diff 19188.Dec 6 2022, 7:30 AM

move handler to Root

Harbormaster completed remote builds in B14067: Diff 19255.Dec 8 2022, 6:47 AM

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 ↗	(On Diff #19255)	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 ↗	(On Diff #19255)	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 ↗	(On Diff #19255)	Can we move `NavigationHandler` inside `NavigationContainer`, and then move `PolicyAcknowledgmentHandler` inside `NavigationHandler`?