Feature/privacy statement cx 1711 #316

stefaniacardenas · 2018-03-05T19:17:23Z

Problem

Users should be aware of the Privacy Policy and Onfido Terms of Use before submitting their documents.

Solution

Added legal documentation view before first document capture to assist client with user privacy notifications and compliance.

The screen will be displayed on both desktop and mobile on the document step
Clicking on Decline will take user back to the previous screen
Once the terms have been accepted the users won't see the screen again
Added tracking

Checklist

put n/a if item is not relevant to PR changes

Have the CHANGELOG, README, MIGRATION, RELEASE_GUIDELINES docs been updated?
Have new automated tests been implemented?
[n/a] Have new manual tests been written down?
Have tests passed locally?
Have any new strings been translated?

…onfido/onfido-sdk-ui into feature/privacy-statement-cx-1711

rfreitas · 2018-03-05T19:20:47Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-05T19:21:05Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T17:44:45Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T17:48:42Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T18:14:58Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T18:15:19Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T19:54:46Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-06T19:54:57Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-07T10:16:48Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-07T16:21:31Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-07T16:26:08Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-07T16:26:36Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

stephencookdev · 2018-03-07T16:28:34Z

src/components/Capture/capture.js

-        onWebcamError: this.onWebcamError,
-        error: this.state.error,
-        ...other}}/>
+      <div>


This wrapping div looks unnecessary, no?

stephencookdev · 2018-03-07T16:32:57Z

src/components/PrivacyStatement/style.css

+@import (less) "../Theme/constants.css";
+
+.privacy {
+  height: calc(~"100% - 112px");


calc isn't a less mixin, or anything - it's a native CSS function

You should just be able to do calc(100% - 112px), right?

what do we need calc for?

We need it because the privacy component has an absolute positioning and we want to make sure that its height never covers the footer so it has a 100% height minus the height (including paddings/margins) of the footer.
@stephencookdev If I use calc(100% - 112px) it gets computed as height: calc(-12%);

so we can't use padding instead? Also, the computation is a bit weird, does padding have a height os 12%?

No, calc does not work as expected with LESS that's why I'm using the following syntax calc(~"100% - 112px"); see this issue.
I cannot use a padding but I could get rid of absolute positioning and give it a fixed height

I added a comment for now as this behaviour is also used in the confirm screen where there is already a comment explaining why we are using calc. This will have to be refactored as part of the header/footer positioning ticket

stephencookdev · 2018-03-07T16:51:59Z

src/components/PrivacyStatement/index.js

+              {i18n.t('privacy.small_print_p1')}
+              <a href='https://onfido.com/termofuse' target='_blank'>{i18n.t('privacy.terms_link')}</a>
+              {i18n.t('privacy.small_print_p2')}
+              <a href='https://onfido.com/privacy' target='_blank'>{i18n.t('privacy.privacy_link')}</a>


This is a really brittle way of doing i18n

You're assuming that any language you want to translate to will fit into the structure you've written here. But you might get a language where it sounds way more natural to order this as "Onfido Terms of Use you are agreeing to" (Japanese is like that, I think)

We're handling this elsewhere by explicitly storing i18n as markdown, and parsing that. But it's fairly heavyweight, we can just get away with it for now. You might want to either make an explicit fragment class, or trust your translations and use dangerouslySetInnerHtml through a translation wrapper (as sort of discussed here)

But also feel free to just kick the bucket down with this one. It'll definitely bite you at some point though, so bear it in mind

That's a very good point. I will have look at fixing this tomorrow.

rfreitas · 2018-03-12T18:37:07Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-13T11:52:02Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-13T11:55:07Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-13T11:56:26Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-13T15:45:14Z

src/components/Capture/capture.js

@@ -43,6 +45,11 @@ class Capture extends Component {
    if (allInvalid) this.onFileGeneralError()
  }

+  acceptTerms = () => {
+    this.props.actions.acceptTerms()
+    this.setState({privacyTermsAccepted: true})


isn't this a prop already?

rfreitas · 2018-03-13T15:45:56Z

src/components/PrivacyStatement/index.js

+
+class PrivacyStatement extends Component {
+  componentDidMount() {
+    sendScreen(['privacy'])


can't just use the tracking component wrapper for this?

We can but the screen name on woopra will be appended to the capture component one, resulting in something like screen_document_front_capture_privacy and we just want screen_privacy

I wonder if we should have as an option to get rid of the prefixes

rfreitas · 2018-03-13T15:46:36Z

src/components/PrivacyStatement/style.css

+@import (less) "../Theme/constants.css";
+
+.privacy {
+  height: calc(~"100% - 112px");


what do we need calc for?

rfreitas · 2018-03-13T15:49:03Z

src/components/Router/index.js

    actions.setDocumentType(documentType)
+    actions.acceptTerms()


can we have a comment on why we are accepting this. also, if the UI order changes and privacy shows after the cross device can be started, this will introduce a bug, which is something to bear in mind.

At the moment, you will never be able to reach cross device if you don't accept the privacy terms, even if you change the steps order. The acceptance criteria didn't say that the privacy screen should not be showed for face capture so I am double checking with @seewah what's the expected behaviour

After our conversation this morning, we agreed on showing the privacy screen also for the face step. Given the current implementation, the terms will always be accepted when the user reaches the cross device client. I could send the termsAccepted prop to the cross device client and trigger this action once we have received it but it seems redundant to me because a user cannot reach the cross device flow without accepting terms first.

# Conflicts: # dist/onfido.min.js

rfreitas · 2018-03-14T13:03:27Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-14T13:07:24Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-14T13:08:35Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-14T17:49:56Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-14T17:52:56Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-14T17:54:25Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

seewah · 2018-03-15T16:03:03Z

webpack.config.babel.js

@@ -90,6 +90,8 @@ const baseStyleRules = (disableExtractToFile = false) =>
 const PROD_CONFIG = {
  'ONFIDO_API_URL': 'https://api.onfido.com',
  'ONFIDO_SDK_URL': 'https://sdk.onfido.com',
+  'ONFIDO_TERMS_URL': 'https://onfido.com/termofuse',


oops sorry my bad it should be /termsofuse (I realise I mistyped termofuse in the ticket!)

seewah · 2018-03-15T16:03:18Z

webpack.config.babel.js

@@ -102,6 +104,8 @@ const TEST_CONFIG = { ...PROD_CONFIG, PUBLIC_PATH: '/', 'MOBILE_URL' : '/' }
 const STAGING_CONFIG = {
  'ONFIDO_API_URL': 'https://apidev.onfido.com',
  'ONFIDO_SDK_URL': 'https://sdk-staging.onfido.com',
+  'ONFIDO_TERMS_URL': 'https://dev.onfido.com/termofuse',


oops sorry my bad it should be /termsofuse (I realise I mistyped termofuse in the ticket!)

seewah · 2018-03-15T17:08:11Z

Apart from my comment re: termsofuse (as opposed to termofuse) I am happy with the ticket. @rfreitas you suggested you would make the change. Do you mind doing that, and I can accept and merge thanks!

rfreitas · 2018-03-16T10:32:14Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-16T10:35:54Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-16T10:37:20Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

seewah · 2018-03-16T12:10:47Z

OK one more issue, on Safari, the onfido logo is obscured on the privacy screen. @stefaniacardenas please work with @danielspagnolo to see what options we have. I see this as a blocker for now.

rfreitas · 2018-03-19T18:44:18Z

Travis automatic deployment: https://staging-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-19T18:44:27Z

Travis automatic deployment: https://release-316-pr-onfido-sdk-ui-onfido.surge.sh

rfreitas · 2018-03-19T18:48:52Z

Travis automatic deployment: https://316-pr-onfido-sdk-ui-onfido.surge.sh

stefaniacardenas added 7 commits February 27, 2018 11:01

Added privacy statement structure

e6d8039

Added terms in store

e470a73

Merge branch 'feature/privacy-statement-cx-1711' of ssh://github.com/…

442f100

…onfido/onfido-sdk-ui into feature/privacy-statement-cx-1711

UI WIP

01c5ee3

More UI changes

5cd3068

Styled privacy screen

eea03ee

Added tests and Spanish translations

747c670

stefaniacardenas requested review from rfreitas and TheComfyChair March 5, 2018 19:17

stefaniacardenas added 2 commits March 6, 2018 16:58

PR feedback from Daniel

7694a40

Fix step covering back button

a96c5ec

Fixed issue with back button

2786f22

Fixed IE11

bee821f

Added automated test for decline

9d47514

stefaniacardenas requested a review from stephencookdev March 7, 2018 16:25

stephencookdev reviewed Mar 7, 2018

View reviewed changes

Embed a links in translation strings

82204f0

PR feedbacks

df83d1f

Add dist

7f35fe6

rfreitas reviewed Mar 13, 2018

View reviewed changes

Removed useless state

43c4d9e

stephencookdev approved these changes Mar 14, 2018

View reviewed changes

Merge branch 'development' into feature/privacy-statement-cx-1711

a33fa8c

# Conflicts: # dist/onfido.min.js

Added a comment about calc

d6c9f6f

rfreitas approved these changes Mar 15, 2018

View reviewed changes

seewah reviewed Mar 15, 2018

View reviewed changes

Fixed terms of use url

bb87869

Fixed margin issue on safari

4e713a2

seewah merged commit 4479088 into development Mar 21, 2018

rfreitas deleted the feature/privacy-statement-cx-1711 branch July 23, 2018 10:40

Feature/privacy statement cx 1711 #316

Feature/privacy statement cx 1711 #316

Conversation

stefaniacardenas commented Mar 5, 2018

Problem

Solution

Checklist

rfreitas commented Mar 5, 2018

rfreitas commented Mar 5, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 6, 2018

rfreitas commented Mar 7, 2018

rfreitas commented Mar 7, 2018

rfreitas commented Mar 7, 2018

rfreitas commented Mar 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfreitas commented Mar 12, 2018

rfreitas commented Mar 13, 2018

rfreitas commented Mar 13, 2018

rfreitas commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfreitas commented Mar 14, 2018

rfreitas commented Mar 14, 2018

rfreitas commented Mar 14, 2018

rfreitas commented Mar 14, 2018

rfreitas commented Mar 14, 2018

rfreitas commented Mar 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seewah commented Mar 15, 2018

rfreitas commented Mar 16, 2018

rfreitas commented Mar 16, 2018

rfreitas commented Mar 16, 2018

seewah commented Mar 16, 2018

rfreitas commented Mar 19, 2018

rfreitas commented Mar 19, 2018

rfreitas commented Mar 19, 2018