-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/privacy statement cx 1711 #316
Changes from 18 commits
e6d8039
e470a73
442f100
01c5ee3
5cd3068
eea03ee
747c670
7694a40
a96c5ec
2786f22
bee821f
9d47514
82204f0
df83d1f
7f35fe6
43c4d9e
a33fa8c
d6c9f6f
bb87869
4e713a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { h, Component} from 'preact' | ||
|
||
import theme from '../Theme/style.css' | ||
import style from './style.css' | ||
import Title from '../Title' | ||
import {preventDefaultOnClick} from '../utils' | ||
import {sendScreen} from '../../Tracker' | ||
import {parseI18nWithXmlTags} from '../../locales' | ||
|
||
const externalUrls = { | ||
terms: process.env.ONFIDO_TERMS_URL, | ||
privacy: process.env.ONFIDO_PRIVACY_URL | ||
} | ||
|
||
class PrivacyStatement extends Component { | ||
componentDidMount() { | ||
sendScreen(['privacy']) | ||
} | ||
|
||
render({i18n, back, acceptTerms}) { | ||
const title = i18n.t('privacy.title') | ||
return ( | ||
<div className={style.privacy}> | ||
<Title {...{title}} /> | ||
<div className={`${theme.thickWrapper} ${style.content}`}> | ||
<ul className={style.list}> | ||
<li className={style.item}>{i18n.t('privacy.item_1')}</li> | ||
<li className={style.item}>{i18n.t('privacy.item_2')}</li> | ||
<li className={style.item}>{i18n.t('privacy.item_3')}</li> | ||
</ul> | ||
|
||
<div> | ||
<div className={style.smallPrint}> | ||
{ parseI18nWithXmlTags(i18n, 'privacy.small_print', tagElement => ( | ||
<a href={externalUrls[tagElement.type]} target='_blank'>{tagElement.text}</a> | ||
))} | ||
</div> | ||
<div className={style.actions}> | ||
<button onClick={preventDefaultOnClick(back)} | ||
className={`${theme.btn} ${style.decline}`}> | ||
{i18n.t('privacy.decline')} | ||
</button> | ||
<button href='#' className={`${theme.btn} ${theme["btn-primary"]} ${style.primary}`} | ||
onClick={preventDefaultOnClick(acceptTerms)}> | ||
{i18n.t('privacy.continue')} | ||
</button> | ||
</div> | ||
</div> | ||
</div> | ||
</div> | ||
) | ||
} | ||
} | ||
|
||
export default PrivacyStatement |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
@import (less) "../Theme/constants.css"; | ||
|
||
.privacy { | ||
/* | ||
The height is calculated by subtracting 112px (parent's margin top 48px + parent's margin bottom (64px)) from the height of its parent. | ||
This will allow consistent spacing between its children when using flexbox | ||
and for the actions to always sit above the "powered-by-onfido" logo. | ||
*/ | ||
height: calc(~"100% - 112px"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You should just be able to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do we need calc for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we can't use padding instead? Also, the computation is a bit weird, does padding have a height os 12%? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
width: 100%; | ||
position: absolute; | ||
} | ||
|
||
.content { | ||
height: calc(~"100% - 112px"); | ||
display: flex; | ||
justify-content: space-around; | ||
flex-direction: column; | ||
color: #0F2536; | ||
} | ||
|
||
.actions { | ||
display: flex; | ||
justify-content: space-between; | ||
flex-direction: row; | ||
} | ||
|
||
.list { | ||
font-size: 16px; | ||
line-height: 24px; | ||
text-align: left; | ||
padding-left: 64px; | ||
position: relative; | ||
@media (--small-viewport) { | ||
padding-left: 32px; | ||
} | ||
} | ||
|
||
.item { | ||
list-style: none; | ||
margin-bottom: 16px; | ||
} | ||
|
||
.item:before{ | ||
content: ''; | ||
display: inline-block; | ||
height: 16px; | ||
width: 32px; | ||
background-image: url('assets/tick-grey.svg'); | ||
background-repeat: no-repeat; | ||
position: relative; | ||
margin-left: -32px; | ||
@media (--small-viewport) { | ||
margin-left: -32px; | ||
} | ||
} | ||
|
||
.smallPrint { | ||
text-align: left; | ||
font-size: 11px; | ||
font-weight: 400; | ||
line-height: 18px; | ||
margin-bottom: 24px; | ||
} | ||
|
||
.smallPrint a { | ||
color: #0F2536; | ||
margin-top: 10px; | ||
margin-bottom: 10px; | ||
} | ||
|
||
.decline { | ||
width: 136px; | ||
border: 1px solid #8A9293; | ||
border-radius: 28px; | ||
background-color: transparent; | ||
color: #585E5F; | ||
line-height: 16px; | ||
@media (--small-viewport) { | ||
width: 112px; | ||
padding: 0px 16px; | ||
} | ||
} | ||
|
||
.primary { | ||
@media (--small-viewport) { | ||
width: 152px; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,9 +98,9 @@ class CrossDeviceMobileRouter extends Component { | |
sendError(`Token has expired: ${token}`) | ||
return this.setError() | ||
} | ||
this.setState({token, steps, step, loading: false}) | ||
this.setState({i18n: initializeI18n(language)}) | ||
this.setState({token, steps, step, loading: false, i18n: initializeI18n(language)}) | ||
actions.setDocumentType(documentType) | ||
actions.acceptTerms() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
setError = () => | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't just use the tracking component wrapper for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 wantscreen_privacy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should have as an option to get rid of the prefixes