Page MenuHomePhabricator

[web] Terms of Use & Privacy modal
ClosedPublic

Authored by kamil on Dec 6 2022, 6:37 AM.
Tags
None
Referenced Files
F3278576: D5829.id19319.diff
Sat, Nov 16, 8:25 AM
F3276049: D5829.id19381.diff
Sat, Nov 16, 7:19 AM
F3276045: D5829.id19182.diff
Sat, Nov 16, 7:18 AM
Unknown Object (File)
Wed, Nov 6, 7:09 PM
Unknown Object (File)
Wed, Nov 6, 6:21 AM
Unknown Object (File)
Tue, Nov 5, 10:00 PM
Unknown Object (File)
Mon, Nov 4, 2:58 PM
Unknown Object (File)
Fri, Nov 1, 4:07 PM
Subscribers

Details

Summary

Modal showing Terms of Use and Privacy Policy acknowledgment.

Design:

Screenshot 2022-12-15 at 12.39.38.png (413×671 px, 30 KB)

Example after server error

Test Plan
  1. I accept should update the value on keyserver DB
  2. User should not be able to close the modal using any method

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil edited the test plan for this revision. (Show Details)
kamil published this revision for review.Dec 6 2022, 8:09 AM
tomek requested changes to this revision.Dec 9 2022, 10:26 AM
tomek added inline comments.
web/modals/terms-and-privacy-modal.css
11 ↗(On Diff #19182)

We can use a shorthand but keeping as is also makes sense as it is more readable

22 ↗(On Diff #19182)

We have a constant for this

web/modals/terms-and-privacy-modal.react.js
38 ↗(On Diff #19182)

We have SetState<T> type and if we used that in acknowledgePolicy definition, we could simplify it here

42–47 ↗(On Diff #19182)

It looks like the modal jumps a little bit after clicking the button. It might be caused by the difference in button size - you can check https://github.com/CommE2E/comm/blob/master/web/settings/relationship/add-users-list.react.js#L220-L227 which is an example of avoiding button size change. Maybe that will help.

53 ↗(On Diff #19182)

We should use callback / a function defined outside the component

This revision now requires changes to proceed.Dec 9 2022, 10:26 AM
kamil edited the test plan for this revision. (Show Details)

improve code, fix modal jumping

tomek added a reviewer: ashoat.

Looks great! Adding @ashoat because this diff adds new copy.

web/modals/terms-and-privacy-modal.css
1–5 ↗(On Diff #19319)

Maybe we should set a line height for the text. It feels like space between lines is a really small and doesn't match https://linear.app/comm/issue/ENG-2350#comment-5f0d2120

On second thought, I think we should revise the copy to:

We recently updated our Terms of Service & Privacy Policy. In order to continue using Comm, we're asking you to read through, acknowledge, and accept the updated policies.

This revision is now accepted and ready to land.Dec 14 2022, 7:01 AM

Please update copy before landing!

update copy, add line height

This revision was automatically updated to reflect the committed changes.