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

Config option to support pre-styled share button #224

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mahemoff
Copy link

This adds the possibility of setting buttonText to false, which will
suppress the default "Share" text being injected into the button.
Benefits: (a) allows the page author to pre-style the button as part of
the general page HTML and CSS; (b) prevents a Flash Of Unwanted Content.

Usage:
new ShareButton(ui: { buttonText: false })

This adds the possibility of setting buttonText to false, which will
suppress the default "Share" text being injected into the button.
Benefits: (a) allows the page author to pre-style the button as part of
the general page HTML and CSS; (b) prevents a Flash Of Unwanted Content.

Usage:
  new ShareButton(ui: { buttonText: false })
@hhsnopek
Copy link
Contributor

hhsnopek commented Oct 9, 2015

@mahemoff when the share-button loads locally I don't get a flash. I'm assuming you want to do something like this:

<share-button>FooBar</share-button>

This would display FooBar instead of Share.

@mahemoff
Copy link
Author

mahemoff commented Oct 9, 2015

@hhsnopek Correct, I'm using this so I can render the contents manually. As far as the flash, if it's generated by JavaScript, there can be a flash if the JavaScript takes a while to load.


// Configure buttonText as false to leave button text alone
var buttonText = this.config.ui.buttonText;
if (buttonText==false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add spacing around the == comparison for me? - this'll be good to go after that 😄

@hhsnopek
Copy link
Contributor

Also you'll have to rebase master on top of this, then we're set to merge 😄 - sorry for the wait!

@mahemoff
Copy link
Author

No worries, should be ready now.

@hhsnopek
Copy link
Contributor

Oops looks like you might have ran git pull origin master pulled without rebasing (-r flag). You should have the merge commits or anything that's already committed 😄

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

Successfully merging this pull request may close these issues.

2 participants