Skip to content

data-url and data-title enhancements#141

Open
epicfaace wants to merge 1 commit intotabalinas:masterfrom
epicfaace:test
Open

data-url and data-title enhancements#141
epicfaace wants to merge 1 commit intotabalinas:masterfrom
epicfaace:test

Conversation

@epicfaace
Copy link
Copy Markdown

Define Share URL and text via data attributes, per issue #108

Comment thread README.md

A string specifying text to share. The content of `<meta name="description">` or `<title>` (if first is missing) is used by default.

The text can also be specified by adding a value for the "data-url" attribute on the sharing element itself.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I guess we want data-title here instead of data-url, right?

Comment thread src/jssocials.js

_initDefaults: function() {
this.url = window.location.href;
this.text = $.trim($("meta[name=description]").attr("content") || $("title").text());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

could we keep defaulting to $("title").text() to avoid breaking changes?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please also keep in mind that twitter has a limit of 140 characters and that the description is actually a description which consists likely out of more then 140 characters.

@tabalinas
Copy link
Copy Markdown
Owner

@epicfaace, thank you for the contribution!
Could you please address a couple of comments?

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