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

DBZ-2619 Adding few social share buttons #620

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

uidoyen
Copy link
Member

@uidoyen uidoyen commented Jan 7, 2021

No description provided.

@uidoyen uidoyen changed the base branch from develop to staging January 7, 2021 16:07
@uidoyen uidoyen changed the base branch from staging to develop January 8, 2021 01:54
@gunnarmorling
Copy link
Member

So I like the idea of share buttons, but can we make them more privacy-friendly? I.e. instead of directly calling those remote URLs, put this behind one more click, so the user consciously opts into this. E.g. see here for one possible solution: https://www.heise.de/extras/socialshareprivacy/ (it's in German, but you'll find similar things described in English, too).

@uidoyen
Copy link
Member Author

uidoyen commented Feb 4, 2021

@gunnarmorling Updated the PR.

@gunnarmorling
Copy link
Member

Hey @uidoyen, nice work overall! A few remarks:

  • Let's remove Tumblr and Stumble, I don't think they're used that widely any more
  • The letter icon seems oddly stretched, can you double check on this?
  • I reckon FaceBook is a plain link and thus can be shown "enabled" right away?

@@ -0,0 +1,154 @@
"use strict";

(function ($,undefined) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this cookie logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

Cookie logic is to enable share buttons permanently.

@gunnarmorling
Copy link
Member

gunnarmorling commented Feb 11, 2021 via email

@uidoyen
Copy link
Member Author

uidoyen commented Feb 15, 2021

Can we do without that permanent feature? We'd need cookie policy disclaimer and option to opt-out for European countries, and it's all not worth the hassle.

Removed permanent feature.

@uidoyen
Copy link
Member Author

uidoyen commented Feb 15, 2021

Hey @uidoyen, nice work overall! A few remarks:

  • Let's remove Tumblr and Stumble, I don't think they're used that widely any more
  • The letter icon seems oddly stretched, can you double check on this?
  • I reckon FaceBook is a plain link and thus can be shown "enabled" right away?

As per panzi/SocialSharePrivacy#50 Facebook is only a link and no request to Facebook is made to display it. Fixed the rest.

@gunnarmorling
Copy link
Member

Hey @uidoyen, could you rebase this and push it to the preview environment if you got some cycles? @hguerrero was just asking for this feature :)

@uidoyen
Copy link
Member Author

uidoyen commented Aug 19, 2021

@gunnarmorling Done!

@gunnarmorling
Copy link
Member

gunnarmorling commented Aug 19, 2021 via email

@uidoyen
Copy link
Member Author

uidoyen commented Aug 20, 2021

Thx! Did you push it to staging, I can't seem to find it there.

@gunnarmorling Just pushed it to staging

@uidoyen
Copy link
Member Author

uidoyen commented Sep 9, 2021

@gunnarmorling @hguerrero Could you please review the staging?

@Naros
Copy link
Member

Naros commented Jul 27, 2022

Hi @uidoyen I'm sorry this has fallen through the cracks. If this is still relevant, could you rebase this PR for me? I'll then take a look and review the changes, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants