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

Add sharing links for the posts #255

Merged
merged 3 commits into from
Feb 6, 2020
Merged

Conversation

mountainbug95
Copy link
Contributor

Fixes #198

@regisphilibert
Copy link
Member

regisphilibert commented Feb 5, 2020

Thanks for thinking about a config but two things:

  1. We need a "disable" sharing config, as I believe by default, user will want those on and we can't expect them to turn on the setting on each post.
  2. I don't think the user will want to limit the number of share services added to their post, for many: the more the better. But if we decide to allow this it should be on the project level. I don't see any reason why user would want Facebook on one post and not the other.

@budparr for 2. do you think we should allow user to disable a given share service? I don't but I might be missing some use cases.

@budparr
Copy link
Member

budparr commented Feb 5, 2020

I agree. I don't think that fine-tuned disabling is necessary

Copy link
Member

@regisphilibert regisphilibert left a comment

Choose a reason for hiding this comment

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

@mountainbug95 Thanks a lot for this! After discussion could you though:

  1. remove the "per post" config and replace it by a disable_share set to false on one example post.
  2. Rework your code for this param test and add all 3 service no matter what.

🙏

layouts/partials/social-share.html Outdated Show resolved Hide resolved
layouts/partials/social-share.html Outdated Show resolved Hide resolved
layouts/partials/social-share.html Outdated Show resolved Hide resolved
layouts/partials/social-share.html Outdated Show resolved Hide resolved
@regisphilibert regisphilibert merged commit 81bc231 into master Feb 6, 2020
@davidsneighbour davidsneighbour deleted the 198-add-share-links branch October 21, 2024 00:34
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.

Add sharing links for the posts
3 participants