Skip to content
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

fix: Add policy pages #1731

Merged
merged 14 commits into from
Mar 7, 2023
Merged

fix: Add policy pages #1731

merged 14 commits into from
Mar 7, 2023

Conversation

usame-algan
Copy link
Member

@usame-algan usame-algan commented Mar 3, 2023

What it solves

Resolves #1722

How this PR fixes it

  • Adds a new page for Privacy policy, Cookie policy, Terms and Imprint
  • Links those pages in the footer
  • Displays the footer on those pages
  • Hides the sidebar on those pages
  • Shows a one time terms popup for users who have previously dismissed the cookie banner (existing users)

How to test it

  1. Open the Safe
  2. Observe a Terms banner showing up in the bottom corner
  3. Open the Safe in a private tab and observe only the cookie banner showing
  4. Accept the cookie banner and observe the Terms banner not showing up
  5. Reload the page and observe the Terms banner not showing up
  6. Go to Settings to see the Footer
  7. Click on any of the three links to see the page
  8. Compare to https://www.notion.so/gnosis-safe/Terms-policy-update-f3827a6f724340deae9554d24afe8c29
  9. Create or Load a new safe and click on terms of use or privacy policy and observe seeing the new pages

Screenshots

Screenshot 2023-03-03 at 11 06 23

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

Branch preview

✅ Deploy successful!

https://add_policy_pages--webcore.review-web-core.5afe.dev

@usame-algan usame-algan mentioned this pull request Mar 3, 2023
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Comment on lines 25 to 27
We updated our Terms and Conditions. Review the new terms{' '}
<Link href={AppRoutes.terms} passHref>
<MUILink color="success.main"> here</MUILink>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
We updated our Terms and Conditions. Review the new terms{' '}
<Link href={AppRoutes.terms} passHref>
<MUILink color="success.main"> here</MUILink>
We've updated our Terms and Conditions. You can review them{' '}
<Link href={AppRoutes.terms} passHref>
<MUILink color="success.main">here</MUILink>.

<Link href={AppRoutes.privacy} passHref>
<MUILink>Privacy Policy</MUILink>
</Link>
, For general web-browsing of this website, your personal data is not revealed to us, although certain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
, For general web-browsing of this website, your personal data is not revealed to us, although certain
, for general web-browsing of this website, your personal data is not revealed to us, although certain

@usame-algan usame-algan force-pushed the add-policy-pages branch 2 times, most recently from 16d7127 to 6e96d83 Compare March 3, 2023 11:25
return (
<div>
<h1>Privacy Policy</h1>
<p>Last updated on March&nbsp;2023.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<p>Last updated on March&nbsp;2023.</p>
<p>Last updated on the 6th of March&nbsp;2023.</p>

Otherwise, the preposition should be in.

@katspaugh
Copy link
Member

Please also bump the minor version in package.json.

@francovenica
Copy link
Contributor

LGTM

@tschubotz
Copy link
Member

When opening for the first time, the cookie banner comes up. The link to the cookie policy is https://safe.global/cookie instead of https://app.safe.global/cookie

image

@tschubotz
Copy link
Member

image

will new users (i.e. users who have never been on the page before) also get this?

(Ideally it would only be displayed to existing users. If we don't have a nice way of determining existing users, I'd say users that have at least 1 safe added.)

@tschubotz
Copy link
Member

I know this wasn't specified anywhere, but I think eventually we should move the licenses also to the app subdomain since it affects the apps only and doesn't make sense on the landing page. Where is the best spot to create an issue for this?

image

@usame-algan
Copy link
Member Author

I know this wasn't specified anywhere, but I think eventually we should move the licenses also to the app subdomain since it affects the apps only and doesn't make sense on the landing page. Where is the best spot to create an issue for this?

I've created a new issue on the web-core board #1737

will new users (i.e. users who have never been on the page before) also get this?

Yes, the Banner would be shown to all users until they click it away.

@tschubotz
Copy link
Member

will new users (i.e. users who have never been on the page before) also get this?

Yes, the Banner would be shown to all users until they click it away.

@usame-algan Is there a way to limit the terms update banner to only existing users?

(If not, am I right to assume that we would leave this up for e.g. 1 month and then disable it again? Would be weird to have this banner up until the end of time.)

@iamacook
Copy link
Member

iamacook commented Mar 6, 2023

@usame-algan Is there a way to limit the terms update banner to only existing users?

We could check the presence of the cookie preferences in localStorage. If the user previously (dis-)agreed to cookies, the key will be there.

@usame-algan
Copy link
Member Author

usame-algan commented Mar 6, 2023

@usame-algan Is there a way to limit the terms update banner to only existing users?

We could check the presence of the cookie preferences in localStorage. If the user previously (dis-)agreed to cookies, the key will be there.

I've changed it so that the Terms banner is only shown if a user previously dismissed the cookie banner. This should cover most existing users.

(If not, am I right to assume that we would leave this up for e.g. 1 month and then disable it again? Would be weird to have this banner up until the end of time.)

We can remove it with a future release.

@francovenica I've updated the description to cover the additional terms banner changes. Could you test it again?

@francovenica
Copy link
Contributor

francovenica commented Mar 6, 2023

I've changed it so that the Terms banner is only shown if a user previously dismissed the cookie banner. This should cover most existing users.

This works, the new terms and conditions message only shows up if you cleared the cookie banner by accepting some or all the preferences. NOTE: It shows up only if you do a full refresh of the page, just going to another section of the app (like settings or assets) won't be enough.

When opening for the first time, the cookie banner comes up. The link to the cookie policy is https://safe.global/cookie instead of https://app.safe.global/cookie

This one was fixed as well

@usame-algan
Copy link
Member Author

This works, the new terms and conditions message only shows up if you cleared the cookie banner by accepting some or all the preferences. NOTE: It shows up only if you do a full refresh of the page, just going to another section of the app (like settings or assets) won't be enough.

I pushed another update to this so that the Terms banner should never show up for new users now, even if they reload the page after accepting cookies.

@francovenica
Copy link
Contributor

TLDR; works fine

So testing this makes me a "new user" every time, since clearing the local storage puts the cookie banner in "not cleared" therefore the LS key for the Terms banner will also be set in "not show".
To be an "old user" I have to remove the new key that shows/hide the Terms banner (as old users should not have that key, so still a realistic case). Reloading the page creates and sets the key again in true, showing me the Terms banner.

@usame-algan usame-algan merged commit 6659e63 into main Mar 7, 2023
@usame-algan usame-algan deleted the add-policy-pages branch March 7, 2023 09:15
@github-actions github-actions bot locked and limited conversation to collaborators Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants