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

WIP: Add accept privacy to the about dialog #437

Merged

Conversation

OlegMoshkovich
Copy link
Member

@OlegMoshkovich OlegMoshkovich commented Oct 14, 2022

This PR mainly contains the change to the about dialog - the slider is replaced by the toggle switch.
It also contains style changes to the dialog component and some style changes to the open dialog.

@netlify
Copy link

netlify bot commented Oct 14, 2022

Deploy Preview for bldrs-share ready!

Name Link
🔨 Latest commit 64c75be
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share/deploys/639791946cee0f00085c2c60
😎 Deploy Preview https://deploy-preview-437--bldrs-share.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@OlegMoshkovich OlegMoshkovich marked this pull request as ready for review October 14, 2022 17:11
@pablo-mayrgundter
Copy link
Member

@MarkusSteinbrecher you brought up in the meeting that you think there's specific language we need for this dialog. Could you research and let us know?

@OlegMoshkovich OlegMoshkovich marked this pull request as draft October 19, 2022 07:25
@MarkusSteinbrecher
Copy link
Contributor

Document is updated with the most relevant content to update our privacy guidelines.

@OlegMoshkovich OlegMoshkovich changed the title Add accept privacy to the about dialog WIP: Add accept privacy to the about dialog Oct 20, 2022
@pablo-mayrgundter
Copy link
Member

I think we're still planning in the design doc. I updated it with last status from Discord. Current agreed proposal is:

  • 1 checkbox, default=checked, for third-party cookies, label TBD
  • Add OK button to dialog
  • No inline mention of functional cookies, but do link to wiki page with detailed info (wiki page TODO)

@OlegMoshkovich
Copy link
Member Author

@pablo-mayrgundter @MarkusSteinbrecher please take a look at the current implementation of the cookies section.

In my view it is strickly better then we have right now. I propose to merge it and continue the discussion about it, and if we agree to change it, it will be done in the separate PR.

@OlegMoshkovich OlegMoshkovich marked this pull request as ready for review October 27, 2022 15:23
@pablo-mayrgundter
Copy link
Member

Heya, thanks for the change. For the label, I suggest:

<h2>Cookies</h2>
Analytics <grey>(anon., 3rd party)</grey> ☑️
<small>(Details...)</small>

@MarkusSteinbrecher
Copy link
Contributor

The radio button is great. Mixed feelings about the colors though but ok ;)

@OlegMoshkovich
Copy link
Member Author

@MarkusSteinbrecher just experimenting a bit ;)
Like the current colors better?

@OlegMoshkovich OlegMoshkovich force-pushed the change_private_to_functional branch 3 times, most recently from 6fee65f to 3114e0e Compare December 9, 2022 09:50
@MarkusSteinbrecher
Copy link
Contributor

MarkusSteinbrecher commented Dec 9, 2022

Looks great. My only question now is: what is on and what is off? With properties we use the radio button opposite to what I would expect. Dark green is turned off and grey is turned on. Is it the same here? Dark green is off? I honestly do not know and maybe I am not the only one?

This is what I am used to regarding radio buttons (we do it the opposite way):
image

@pablo-mayrgundter
Copy link
Member

Agreed about the colors.. given a bright active color and a mute passive color, the bright color should indicate on, passive off.

For the text, please call it "Analytics cookies (read more)" and link "read more" here: https://github.com/bldrs-ai/Share/wiki/Design#privacy

Copy link
Member Author

@OlegMoshkovich OlegMoshkovich left a comment

Choose a reason for hiding this comment

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

Made the changes to the switch track, added accept cookies title, added read more link.

@pablo-mayrgundter
Copy link
Member

pablo-mayrgundter commented Dec 12, 2022

Looks good, but I think we want it on by default! :)

EDIT: n/m.. my mistake. Looks good

@OlegMoshkovich OlegMoshkovich merged commit 854c3a8 into bldrs-ai:main Dec 13, 2022
@OlegMoshkovich OlegMoshkovich deleted the change_private_to_functional branch March 29, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants