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

Social Sharing: Move mailto functionality from js to html #1281

Merged
merged 2 commits into from
Aug 30, 2016

Conversation

rclations
Copy link

per #799, this PR removes the js code that builds the mailto link, and adds it to the template files instead.

@@ -88,7 +88,13 @@ function largo_post_social_links( $echo = true ) {
}

if ( $utilities['email'] === '1' ) {
$output .= '<span data-service="email" class="email custom-share-button share-button"><a><i class="icon-mail"></i> <span class="hidden-phone">' . esc_attr( __( 'Email', 'largo' ) ) . '</span></a></span>';
$output .= sprintf(
'<span data-service="email" class="email share-button"><a href="mailto:?subject=%2$s&body=%3$s%0D%0A%4$s" target="_blank"><i class="icon-mail-alt"></i> <span class="hidden-phone">%1$s</span></a></span>',
Copy link
Author

Choose a reason for hiding this comment

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

Just a quick note on this -

The body field still opens in Apple Mail with the post URL appended to the end of the excerpt (no space in-between), but I don't believe there's any way around this.

We could add a <br /> tag, but that's not the correct way to do that, and it would likely have mixed results in browser-driven email applications.

For more info, see http://stackoverflow.com/questions/22765834/insert-a-line-break-in-mailto-body#22765878

$output .= '<span data-service="email" class="email custom-share-button share-button"><a><i class="icon-mail"></i> <span class="hidden-phone">' . esc_attr( __( 'Email', 'largo' ) ) . '</span></a></span>';
$output .= sprintf(
'<span data-service="email" class="email share-button"><a href="mailto:?subject=%2$s&body=%3$s%0D%0A%4$s" target="_blank"><i class="icon-mail-alt"></i> <span class="hidden-phone">%1$s</span></a></span>',
esc_attr( __( 'Email', 'npr' ) ),

Choose a reason for hiding this comment

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

can't edit your fork but need to use the largo textdomain here instead of npr

@aschweigert
Copy link

other than that one textdomain issue this lgtm

@aschweigert aschweigert merged commit ba203bd into WPBuddy:develop Aug 30, 2016
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