-
Notifications
You must be signed in to change notification settings - Fork 194
Allow separate control of sharing in blog post and main menu #260
base: master
Are you sure you want to change the base?
Conversation
Update translation to match current view
- add parameter to control sharing from post body separately from header menu - move sharing button to the bottom of the post and add title to make it clear it's sharing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I like the idea, but maybe not this solution. It doesn't really solve the problem, it just moves them to where it bothers you less until the next person that comes along and wants them moved to the top.
I do agree that we could probably transition these icons to be more muted, or even drop them all for the share button that is in the header.
@@ -28,6 +28,7 @@ disableLanguages = [] | |||
imageStretch = "" | |||
removeBlur = false | |||
socialShare = ["twitter", "facebook", "reddit", "linkedin", "pinterest", "email"] | |||
shareFromPost = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to the config.toml
should not change what legacy users expect if at all possible. Since this must be set as true, users that simply update the theme would be missing the share buttons if they didn't add this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to avoid this incompatibility with a new parameter?
The only one I can think of is to change it to disableSharingFromPost
or disablePostSharing
but will be somewhat inconsistent with all other parameters.
Let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -37,7 +37,7 @@ <h1 class="nav-title"> | |||
{{ if .Site.Params.header.languageMenu }}{{ partial "language-menu" . }}{{ end }} | |||
{{ if .Site.Params.header.shareMenu }} | |||
<menu id="share-menu" class="flyout-menu menu"> | |||
<h1>Share Post</h1> | |||
<h1>{{ i18n "share_post" }}</h1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure why this wasn't i18n before, but we need to make sure its in all languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've counted the number of occurrences of share_post
in our codebase and the number of l10n files. Both are 13. The former should be strictly greater than the later coz this param is called in the Go-HTML template code. The -A <N>
flag can be appended to grep
to show <N>
lines after the matched pattern.
$ find i18n -type f
i18n/fr.toml
i18n/zh-CN.toml
i18n/ja.toml
i18n/tr.toml
i18n/en.toml
i18n/ko.toml
i18n/id.toml
i18n/pl.toml
i18n/es.toml
i18n/nl.toml
i18n/de.toml
i18n/zh-TW.toml
i18n/pt.toml
$ find i18n -type f | wc -l
13
$ git grep share_post | wc -l
13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right, the localized string was not used before. So I put the actual string as it appeared in browser to the i18n file. What's next? Do you want me to isolate this change from the share button update? Or you'll fix it? Or you want to start using the localized version but the longer from from the i18n/en?
Show sharing buttons when the config parameter is missing for backward compatibility
Description
Motivation and Context
It was impossible to enable sharing from the main menu, but disable in the post.
Side note: Buttons in the post don't really match the light style of the theme, they're quite big and better be placed below the main content.
Screenshots (if appropriate):
The buttons are the same, but I'm not the expert in html/css. Maybe it's better to redesign them completely and make less obtrusive (e.g see reddit, it has a single small nice share button with pop-up menu).
Feel free to reach out to me in telegram if you want to discuss any of PRs in real-time
Checklist:
theme.toml
, as applicable.