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

chore(cookie-policy): reactivate the cookie policy banner (DSP-1727) #461

Merged
merged 3 commits into from Jun 11, 2021

Conversation

@kilchenmann
Copy link
Collaborator

@kilchenmann kilchenmann commented Jun 10, 2021

resolves DSP-1727

@kilchenmann kilchenmann requested review from mdelez, subotic and waychal Jun 10, 2021
@kilchenmann kilchenmann self-assigned this Jun 10, 2021
@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 10, 2021

The "history back" button doesn't work anymore. Will fix it...
--> the cookie policy were always opened in a new tab because of the external-links.directive. This is why the back button didn't work. Resolved in f02bb21

Loading

@kilchenmann kilchenmann requested review from mdelez, subotic and waychal Jun 10, 2021
Copy link
Contributor

@mdelez mdelez left a comment

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

Loading

<!-- cookie information banner -->
<div class="cookie-banner" *ngIf="showCookieBanner">
<p class="note">
This web-application uses cookies to provide you with great user experience. By using the application you
Copy link
Contributor

@mdelez mdelez Jun 11, 2021

Choose a reason for hiding this comment

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

with a greater user experience

Loading

Copy link
Collaborator Author

@kilchenmann kilchenmann Jun 11, 2021

Choose a reason for hiding this comment

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

rewritten in fd2ecf2

Loading

@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 11, 2021

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

But what should happen, when the user click on "reject"? Redirect the user to a google page? Because in this case s/he can't use the app.
This is anyway a temporary solution only. Daniela and Marchin are working on a better implementation, where the user can select which cookies should be enabled.

Loading

@kilchenmann kilchenmann requested a review from mdelez Jun 11, 2021
@mdelez
Copy link
Contributor

@mdelez mdelez commented Jun 11, 2021

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

But what should happen, when the user click on "reject"? Redirect the user to a google page? Because in this case s/he can't use the app.
This is anyway a temporary solution only. Daniela and Marchin are working on a better implementation, where the user can select which cookies should be enabled.

The website should still be useable even if the user rejected all non-essential cookies. We can still use cookie that are essential to the website operating. The user just needs to be allowed the reject any cookie that may have identifying data. https://gdpr.eu/cookies/#:~:text=To%20comply%20with%20the%20regulations,cookies%20except%20strictly%20necessary%20cookies.&text=Allow%20users%20to%20access%20your,the%20use%20of%20certain%20cookies

Loading

@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 11, 2021

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

But what should happen, when the user click on "reject"? Redirect the user to a google page? Because in this case s/he can't use the app.
This is anyway a temporary solution only. Daniela and Marchin are working on a better implementation, where the user can select which cookies should be enabled.

The website should still be useable even if the user rejected all non-essential cookies. We can still use cookie that are essential to the website operating. The user just needs to be allowed the reject any cookie that may have identifying data. https://gdpr.eu/cookies/#:~:text=To%20comply%20with%20the%20regulations,cookies%20except%20strictly%20necessary%20cookies.&text=Allow%20users%20to%20access%20your,the%20use%20of%20certain%20cookies

Yes I know and you're right, but in DSP-APP we have only essential cookies (the one for the login). When the user reject them s/he can't use the app. This is why I wrote: "If you want to use the app, you have to accept the cookie policy".

Loading

@mdelez
Copy link
Contributor

@mdelez mdelez commented Jun 11, 2021

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

But what should happen, when the user click on "reject"? Redirect the user to a google page? Because in this case s/he can't use the app.
This is anyway a temporary solution only. Daniela and Marchin are working on a better implementation, where the user can select which cookies should be enabled.

The website should still be useable even if the user rejected all non-essential cookies. We can still use cookie that are essential to the website operating. The user just needs to be allowed the reject any cookie that may have identifying data. https://gdpr.eu/cookies/#:~:text=To%20comply%20with%20the%20regulations,cookies%20except%20strictly%20necessary%20cookies.&text=Allow%20users%20to%20access%20your,the%20use%20of%20certain%20cookies

Yes I know and you're right, but in DSP-APP we have only essential cookies (the one for the login). When the user reject them s/he can't use the app. This is why I wrote: "If you want to use the app, you have to accept the cookie policy".

That's fair. So then at that point I don't think we even need a cookies banner. You could also just add a "Reject non-essential cookies" button that does nothing but dismiss the banner since we don't have any non-essential cookies.

Loading

@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 11, 2021

I think there needs to be a "Reject" button as well, no? There is currently no way to dismiss the banner besides clicking the "Accept" button.

But what should happen, when the user click on "reject"? Redirect the user to a google page? Because in this case s/he can't use the app.
This is anyway a temporary solution only. Daniela and Marchin are working on a better implementation, where the user can select which cookies should be enabled.

The website should still be useable even if the user rejected all non-essential cookies. We can still use cookie that are essential to the website operating. The user just needs to be allowed the reject any cookie that may have identifying data. https://gdpr.eu/cookies/#:~:text=To%20comply%20with%20the%20regulations,cookies%20except%20strictly%20necessary%20cookies.&text=Allow%20users%20to%20access%20your,the%20use%20of%20certain%20cookies

Yes I know and you're right, but in DSP-APP we have only essential cookies (the one for the login). When the user reject them s/he can't use the app. This is why I wrote: "If you want to use the app, you have to accept the cookie policy".

That's fair. So then at that point I don't think we even need a cookies banner. You could also just add a "Reject non-essential cookies" button that does nothing but dismiss the banner since we don't have any non-essential cookies.

As @subotic said yesterday, I should reactivate the banner. So that's what I did.

Loading

mdelez
mdelez approved these changes Jun 11, 2021
@kilchenmann
Copy link
Collaborator Author

@kilchenmann kilchenmann commented Jun 11, 2021

Thank you @mdelez

Loading

@kilchenmann kilchenmann merged commit ac99fbc into main Jun 11, 2021
8 checks passed
Loading
@kilchenmann kilchenmann deleted the wip/dsp-1727-cookie-policy branch Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants