mxstbr / sharingbuttons.io

Quickly generate social sharing buttons with a tiny performance footprint
http://sharingbuttons.io
MIT License
2.48k stars 103 forks source link

Add vk button #73

Closed aghh1504 closed 7 years ago

aghh1504 commented 7 years ago

fixes #46 Also added new line character to Whatsapp SVG section.

mxstbr commented 7 years ago

This looks great! 👏

One tiny thing, embedding the VK share script actually does 3 requests and fetches 6.3kb. Could you update those values?

I tested this by creating a index.html file with these contents:

<!DOCTYPE html>
<html>
    <head>
        <meta charset="utf-8">
        <title>Test</title>
        <!-- Put this script tag to the <head> of your page -->
        <script type="text/javascript" src="https://vk.com/js/api/share.js?94" charset="windows-1251"></script>

    </head>
    <body>
        <!-- Put this script tag to the place, where the Share button will be -->
        <script type="text/javascript"><!--
        document.write(VK.Share.button(false,{type: "round", text: "Share"}));
        --></script>
    </body>
</html>

(this was just copied from the VK share button site above)

This is what I see in the DevTools network tab:

screen shot 2016-12-03 at 17 03 19

As you can see it loads the script itself, which injects another script tag into the HEAD and fetches an image. All together that's three requests that weigh 6.3kb!

mxstbr commented 7 years ago

Now that we've merged the HackerNews support in #74 you'll need to rebase on top of master. Sorry for the troubles! (this video should help in case you've never rebased before!)

mxstbr commented 7 years ago

Deploy preview failed.

Built with commit 2df31fa91a8dc652069e01cba9f2ddc505ed17fb

https://app.netlify.com/sites/sharingbuttons/deploys/5842f69ed6865d54ae504ff9

mxstbr commented 7 years ago

Deploy preview failed.

Built with commit 96147e8809246c0e22477e7fb78a12d250d70340

https://app.netlify.com/sites/sharingbuttons/deploys/5842fa24d6865d2ad9504ff1

mxstbr commented 7 years ago

The deploy error seems to be on Netlify's side, your code looks great! This is the log I get:

6:00:20 PM: Build started
6:00:20 PM: API communication error. Canceling build.

Sounds like Netlify is down?! 😱

Anyhow, thanks very much, I'll merge this! Well done!!

richardwestenra commented 7 years ago

Hey @mxstbr, just wanted to drop in and say thanks a lot for the timely and welcoming review comments 🙂. I worked with @aghh1504 on this PR today at @codebar's 24PRs event. Thanks for the friendly guidance and feedback!

mxstbr commented 7 years ago

It seems like Netlify is still having issues, as I'm still not seeing your great VK button on sharingbuttons.io 😭

I've reached out to their support team, I hope we'll have this resolved very soon. Sorry for the delay in getting your feature live!

mxstbr commented 7 years ago

Netlify is back, your VK button is now live! 🎉

http://sharingbuttons.io/

screen shot 2016-12-04 at 09 40 49

Thanks so much!

aghh1504 commented 7 years ago

Thats great;) happy to help