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

Feature: A better share button NeedMoreShare2 #1913

Merged
merged 34 commits into from
Oct 4, 2017
Merged

Feature: A better share button NeedMoreShare2 #1913

merged 34 commits into from
Oct 4, 2017

Conversation

LEAFERx
Copy link
Contributor

@LEAFERx LEAFERx commented Sep 30, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs have been added / updated (for bug fixes / features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the new behavior?

Add a new share button which is a pure javascript lib.
It will be the only sharebutton that supports HTTPS in NexT (for now) and it has a better UI.
It's useful in China.
https://github.com/revir/need-more-share2

  • Screens with this changes:

1 Desktop Postbottom

image 1
when hover:
image 2
when click:
image 3

2 Desktop Float

image 4
when hover:
image 5
when click:
image 6

3 Mobile Postbottom

image 11
image 12

4 Mobile Float

image 9
image 10

How to use?

In NexT _config.yml:

# NeedMoreShare2
# This plugin is a pure javascript sharing lib which is useful in China.
# See: https://github.com/revir/need-more-share2
needMoreShare2:
  postbottom: 
    enable: true
    Options:
      iconStyle: box
      boxForm: horizontal
      position: middleCenter
      networks: Weibo,Wechat,Douban,QQZone,Twitter,Facebook
  float: 
    enable: true
    Options:
      iconStyle: box
      boxForm: horizontal
      position: middleRight
      networks: Weibo,Wechat,Douban,QQZone,Twitter,Facebook

Does this PR introduce a breaking change?

  • Yes.
  • No.

@@ -0,0 +1 @@
!function(){function a(a,b){if("string"==typeof b){var c=a.matches||a.webkitMatchesSelector||a.mozMatchesSelector||a.msMatchesSelector;if(c)for(;a;){if(c.bind(a)(b))return a;a=a.parentElement}return!1}for(;a;){if(a==b)return a;a=a.parentElement}return!1}window.needShareButton=function(b,c){function d(a){var b={};for(var c in f.options)b[c]=f.options[c];b.url=window.location.href,b.title=f.getTitle(),b.image=f.getImage(),b.description=f.getDescription();for(var d in a.dataset)if(d.match(/share/)){var e=d.replace(/share/,"");if(!e.length)continue;e=e.charAt(0).toLowerCase()+e.slice(1);var g=a.dataset[d];"networks"===e?g=g.toLowerCase().split(","):"url"===e&&g&&"/"===g[0]&&(g=location.origin+g),b[e]=g}return b}function e(b){var c=document.createElement("span");if(c.className="need-share-button_dropdown",!b.querySelector(".need-share-button_dropdown")){var e=d(b);"box"==e.iconStyle&&"horizontal"==e.boxForm?c.className+=" need-share-button_dropdown-box-horizontal":"box"==e.iconStyle&&"vertical"==e.boxForm&&(c.className+=" need-share-button_dropdown-box-vertical"),setTimeout(function(){switch(e.position){case"topLeft":c.className+=" need-share-button_dropdown-top-left";break;case"topRight":c.className+=" need-share-button_dropdown-top-right";break;case"topCenter":c.className+=" need-share-button_dropdown-top-center",c.style.marginLeft=-c.offsetWidth/2+"px";break;case"middleLeft":c.className+=" need-share-button_dropdown-middle-left",c.style.marginTop=-c.offsetHeight/2+"px";break;case"middleRight":c.className+=" need-share-button_dropdown-middle-right",c.style.marginTop=-c.offsetHeight/2+"px";break;case"bottomLeft":c.className+=" need-share-button_dropdown-bottom-left";break;case"bottomRight":c.className+=" need-share-button_dropdown-bottom-right";break;case"bottomCenter":c.className+=" need-share-button_dropdown-bottom-center",c.style.marginLeft=-c.offsetWidth/2+"px";break;default:c.className+=" need-share-button_dropdown-bottom-center",c.style.marginLeft=-c.offsetWidth/2+"px"}},1);var g="default"==e.iconStyle?"need-share-button_link need-share-button_":"need-share-button_link-"+e.iconStyle+" need-share-button_link need-share-button_";for(var h in e.networks){var i=document.createElement("span");h=e.networks[h],i.className=g+h,i.className+=" icon-"+h,i.dataset.network=h,i.title=h,c.appendChild(i)}c.addEventListener("click",function(c){if(a(c.target,".need-share-button_link"))return c.preventDefault(),c.stopPropagation(),f.share[c.target.dataset.network](b),!1}),b.appendChild(c)}}var f=this;f.elem=b||"need-share-button",f.getTitle=function(){var a;return document.querySelector&&(a=document.querySelector("title"))?a.innerText:document.title},f.getImage=function(){var a;return document.querySelector&&(a=document.querySelector('meta[property="og:image"]')||document.querySelector('meta[name="twitter:image"]'))?a.getAttribute("content"):""},f.getDescription=function(){var a;return document.querySelector?(a=document.querySelector('meta[property="og:description"]')||document.querySelector('meta[name="twitter:description"]')||document.querySelector('meta[name="description"]'))?a.getAttribute("content"):"":(a=document.getElementsByTagName("meta").namedItem("description"))?a.getAttribute("content"):""},f.share={weibo:function(a){var b=d(a),c="http://v.t.sina.com.cn/share/share.php?title="+encodeURIComponent(b.title)+"&url="+encodeURIComponent(b.url)+"&pic="+encodeURIComponent(b.image);f.popup(c)},wechat:function(a){var b=d(a),c="https://api.qinco.me/api/qr?size=400&content="+encodeURIComponent(b.url),e=a.querySelector(".need-share-button_dropdown"),f=e.getElementsByClassName("need-share-wechat-code-image")[0];f?f.remove():(f=document.createElement("img"),f.src=c,f.alt="loading wechat image...",f.setAttribute("class","need-share-wechat-code-image"),e.appendChild(f))},douban:function(a){var b=d(a),c="https://www.douban.com/share/service?name="+encodeURIComponent(b.title)+"&href="+encodeURIComponent(b.url)+"&image="+encodeURIComponent(b.image);f.popup(c)},qqzone:function(a){var b=d(a),c="http://sns.qzone.qq.com/cgi-bin/qzshare/cgi_qzshare_onekey?title="+encodeURIComponent(b.title)+"&url="+encodeURIComponent(b.url)+"&pics="+encodeURIComponent(b.image)+"&desc="+encodeURIComponent(b.description);f.popup(c)},renren:function(a){var b=d(a),c="http://widget.renren.com/dialog/share?title="+encodeURIComponent(b.title)+"&resourceUrl="+encodeURIComponent(b.url)+"&pic="+encodeURIComponent(b.image)+"&description="+encodeURIComponent(b.description);f.popup(c)},mailto:function(a){var b=d(a),c="mailto:?subject="+encodeURIComponent(b.title)+"&body=Thought you might enjoy reading this: "+encodeURIComponent(b.url)+" - "+encodeURIComponent(b.description);window.location.href=c},twitter:function(a){var b=d(a),c=b.protocol+"twitter.com/intent/tweet?text=";c+=encodeURIComponent(b.title)+"&url="+encodeURIComponent(b.url),f.popup(c)},pinterest:function(a){var b=d(a),c=b.protocol+"pinterest.com/pin/create/bookmarklet/?is_video=false";c+="&media="+encodeURIComponent(b.image),c+="&url="+encodeURIComponent(b.url),c+="&description="+encodeURIComponent(b.title),f.popup(c)},facebook:function(a){var b=d(a),c=b.protocol+"www.facebook.com/share.php?";c+="u="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},googleplus:function(a){var b=d(a),c=b.protocol+"plus.google.com/share?";c+="url="+encodeURIComponent(b.url),f.popup(c)},reddit:function(a){var b=d(a),c=b.protocol+"www.reddit.com/submit?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},delicious:function(a){var b=d(a),c=b.protocol+"del.icio.us/post?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&notes="+encodeURIComponent(b.description),f.popup(c)},tapiture:function(a){var b=d(a),c=b.protocol+"tapiture.com/bookmarklet/image?";c+="img_src="+encodeURIComponent(b.image),c+="&page_url="+encodeURIComponent(b.url),c+="&page_title="+encodeURIComponent(b.title),f.popup(c)},stumbleupon:function(a){var b=d(a),c=b.protocol+"www.stumbleupon.com/submit?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},linkedin:function(a){var b=d(a),c=b.protocol+"www.linkedin.com/shareArticle?mini=true";c+="&url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&source="+encodeURIComponent(b.source),f.popup(c)},slashdot:function(a){var b=d(a),c=b.protocol+"slashdot.org/bookmark.pl?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},technorati:function(a){var b=d(a),c=b.protocol+"technorati.com/faves?";c+="add="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},posterous:function(a){var b=d(a),c=b.protocol+"posterous.com/share?";c+="linkto="+encodeURIComponent(b.url),f.popup(c)},tumblr:function(a){var b=d(a),c=b.protocol+"www.tumblr.com/share?v=3";c+="&u="+encodeURIComponent(b.url),c+="&t="+encodeURIComponent(b.title),f.popup(c)},googlebookmarks:function(a){var b=d(a),c=b.protocol+"www.google.com/bookmarks/mark?op=edit";c+="&bkmk="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&annotation="+encodeURIComponent(b.description),f.popup(c)},newsvine:function(a){var b=d(a),c=b.protocol+"www.newsvine.com/_tools/seed&save?";c+="u="+encodeURIComponent(b.url),c+="&h="+encodeURIComponent(b.title),f.popup(c)},pingfm:function(a){var b=d(a),c=b.protocol+"ping.fm/ref/?";c+="link="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&body="+encodeURIComponent(b.description),f.popup(c)},evernote:function(a){var b=d(a),c=b.protocol+"www.evernote.com/clip.action?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},friendfeed:function(a){var b=d(a),c=b.protocol+"www.friendfeed.com/share?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),f.popup(c)},vkontakte:function(a){var b=d(a),c=b.protocol+"vkontakte.ru/share.php?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&description="+encodeURIComponent(b.description),c+="&image="+encodeURIComponent(b.image),c+="&noparse=true",f.popup(c)},odnoklassniki:function(a){var b=d(a),c=b.protocol+"www.odnoklassniki.ru/dk?st.cmd=addShare&st.s=1";c+="&st.comments="+encodeURIComponent(b.description),c+="&st._surl="+encodeURIComponent(b.url),f.popup(c)},mailru:function(a){var b=d(a),c=b.protocol+"connect.mail.ru/share?";c+="url="+encodeURIComponent(b.url),c+="&title="+encodeURIComponent(b.title),c+="&description="+encodeURIComponent(b.description),c+="&imageurl="+encodeURIComponent(b.image),f.popup(c)}},f.popup=function(a){var b=600,c=500,d=void 0!=window.screenLeft?window.screenLeft:screen.left,e=void 0!=window.screenTop?window.screenTop:screen.top,f=window.innerWidth?window.innerWidth:document.documentElement.clientWidth?document.documentElement.clientWidth:screen.width,g=window.innerHeight?window.innerHeight:document.documentElement.clientHeight?document.documentElement.clientHeight:screen.height,h=f/2-b/2+d,i=g/2-c/2+e,j=window.open(a,"targetWindow","toolbar=no,location=no,status=no,menubar=no,scrollbars=yes,resizable=yes,width="+b+", height="+c+", top="+i+", left="+h);window.focus&&j.focus()},f.options={iconStyle:"default",boxForm:"horizontal",position:"bottomCenter",protocol:["http","https"].indexOf(window.location.href.split(":")[0])===-1?"https://":"//",networks:"Weibo,Wechat,Douban,QQZone,Twitter,Pinterest,Facebook,GooglePlus,Reddit,Linkedin,Tumblr,Evernote"};for(var g in c)f.options[g]=c[g];f.options.networks=f.options.networks.toLowerCase().split(","),document.addEventListener("click",function(b){var c=document.querySelector(".need-share-button-opened");if(!a(b.target,".need-share-button-opened"))if(c)c.classList.remove("need-share-button-opened"),c.querySelector(".need-share-wechat-code-image")&&c.querySelector(".need-share-wechat-code-image").remove();else{var d=a(b.target,f.elem);d&&(d.classList.contains("need-share-button-opened")||(e(d),setTimeout(function(){d.classList.add("need-share-button-opened")},1)))}})},new needShareButton(".need-share-button")}();

Choose a reason for hiding this comment

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

Line is too long.
Expected '{' and instead saw 'for'.
Expected '{' and instead saw 'return'.
Missing semicolon.
Expected '{' and instead saw 'b'.
The body of a for in should be wrapped in an if statement to filter unwanted properties from the prototype.
Expected '{' and instead saw 'if'.
Expected '{' and instead saw 'continue'.
'f' was used before it was defined.
Mixed double and single quotes.
Too many errors. (100% scanned).

@@ -5,3 +5,4 @@
@import "localsearch";
@import "busuanzi-counter";
@import "algolia-search" if hexo-config('algolia_search.enable');
@import "needsharebutton";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add conditional import here.

_config.yml Outdated
# This plugin is a pure javascript sharing lib which is useful in China.
# See: https://github.com/revir/need-more-share2
needMoreShare2:
postbottom:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add global enable/disable switch for better control it.

@ivan-nginx
Copy link
Collaborator

@LEAFERx and can u show live demo on it? Why on your blog i don't see this feature?

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Sep 30, 2017

@ivan-nginx now it's on my blog

Copy link
Collaborator

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

Please, use lowercases in main config.

_config.yml Outdated
enable: false
postbottom:
enable: false
Options:
Copy link
Collaborator

Choose a reason for hiding this comment

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

To lowercase?

_config.yml Outdated
networks: Weibo,Wechat,Douban,QQZone,Twitter,Facebook
float:
enable: false
Options:
Copy link
Collaborator

Choose a reason for hiding this comment

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

To lowercase?

_config.yml Outdated
# NeedMoreShare2
# This plugin is a pure javascript sharing lib which is useful in China.
# See: https://github.com/revir/need-more-share2
needMoreShare2:
Copy link
Collaborator

Choose a reason for hiding this comment

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

To lowercase? (_)

@ivan-nginx
Copy link
Collaborator

Also, need to test it on other schemes (Pisces | Gemini).

Copy link
Collaborator

@ivan-nginx ivan-nginx left a comment

Choose a reason for hiding this comment

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

Just -float and -postbottom need to add after settings compare. No need to excess strings of code.

@@ -0,0 +1 @@
<div id="need-share-button-float"><i class="fa fa-share-alt" aria-hidden="true"></i></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

And can u please to split this 2 files into 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is ok to use macro to identify whether it is float or postbottom?And where should i import the macro in _layout.swig and post.swig?

Copy link
Collaborator

Choose a reason for hiding this comment

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

U joking? This is floatdom.swig

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will make a new file called dom.swig like this:

{% macro render type %}
    <span id="needsharebutton-{{type}}"><i class="fa fa-share-alt" aria-hidden="true"></i></span>
{% endmacro %}

(maybe i reply in the wrong place...)

Copy link
Collaborator

@ivan-nginx ivan-nginx Sep 30, 2017

Choose a reason for hiding this comment

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

Seems good. But why filename dom.swig? I think filename may be simple needsharebutton.swig.
layout/_third-party/needsharebutton.swig

@@ -0,0 +1 @@
<span id="need-share-button-postbottom"><i class="fa fa-share-alt" aria-hidden="true"></i></span>
Copy link
Collaborator

Choose a reason for hiding this comment

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

And can u please to split this 2 files into 1?

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Sep 30, 2017

Also, need to add in description all supported networks from needShareButton:

default: 'Mailto,Twitter,Pinterest,Facebook,GooglePlus,Reddit,Delicious,Tapiture,StumbleUpon,Linkedin,Slashdot,Technorati,Posterous,Tumblr,GoogleBookmarks,Newsvine,Pingfm,Evernote,Friendfeed,Vkontakte,Odnoklassniki,Mailru'
options: Any of the network name's from pervios row comma separated

So, this plugin not only for China useful. 😃

_config.yml Outdated
iconStyle: box
boxForm: horizontal
position: middleCenter
networks: Weibo,Wechat,Douban,QQZone,Twitter,Facebook
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mailto,Twitter,Pinterest,Facebook,GooglePlus,Reddit,Delicious,Tapiture,StumbleUpon,Linkedin,Slashdot,Technorati,Posterous,Tumblr,GoogleBookmarks,Newsvine,Pingfm,Evernote,Friendfeed,Vkontakte,Odnoklassniki,Mailru

_config.yml Outdated
iconStyle: box
boxForm: horizontal
position: middleRight
networks: Weibo,Wechat,Douban,QQZone,Twitter,Facebook
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mailto,Twitter,Pinterest,Facebook,GooglePlus,Reddit,Delicious,Tapiture,StumbleUpon,Linkedin,Slashdot,Technorati,Posterous,Tumblr,GoogleBookmarks,Newsvine,Pingfm,Evernote,Friendfeed,Vkontakte,Odnoklassniki,Mailru

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Sep 30, 2017

Also, take a look on this screen:
image

As u can see, there is main widget what include:

  1. Social rating.
  2. VK likes.
  3. FB likes & shares.

And i think will be better if we include this new share plugin into post-widgets.styl and same block.
What u think about this?


@import "post-widgets" if (hexo-config('facebook_sdk.enable') and hexo-config('facebook_sdk.like_button')) or (hexo-config('vkontakte_api.enable') and hexo-config('vkontakte_api.like')) or hexo-config('rating.enable');

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Sep 30, 2017

OK. I'll try.

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Sep 30, 2017

1 lowercase in config
2 put everything into needsharebutton.swig and needsharebutton.styl
3 rewrite the description
4 put the postbottom button into the post-widget
5 test it in Pisces&Gemini and find it works in the right way

@ivan-nginx
Copy link
Collaborator

Ok, i'll test it now too, wait a moment...

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Sep 30, 2017

needmoreshare2:
  enable: true
  postbottom: 
    enable: true
    options:
      iconStyle: box
      boxForm: horizontal
      position: bottomCenter
      networks: Weibo,Wechat,Douban,QQZone,Twitter,Linkedin,Mailto,Twitter,Reddit,Delicious,Tapiture,StumbleUpon,Pinterest,Facebook,GooglePlus,Linkedin,Slashdot,Technorati,Posterous,Tumblr,GoogleBookmarks,Newsvine,Pingfm,Evernote,Friendfeed,Vkontakte,Odnoklassniki,Mailru
  float: 
    enable: true
    options:
      iconStyle: box
      boxForm: horizontal
      position: middleRight
      networks: Weibo,Wechat,Douban,QQZone,Twitter,Linkedin,Mailto,Twitter,Reddit,Delicious,Tapiture,StumbleUpon,Pinterest,Facebook,GooglePlus,Linkedin,Slashdot,Technorati,Posterous,Tumblr,GoogleBookmarks,Newsvine,Pingfm,Evernote,Friendfeed,Vkontakte,Odnoklassniki,Mailru

image
image

It seems some css styles need to improvement.


EDITED: it's mb positions need to define to various icons?


U may compare version2 and original. In Chinese version there is no icons for email, for example. This is not good. Need to do for all, not for Chinese only. Why he's cutting this icons? Economy for few strings of code or what? 😄

revir/need-more-share2@0e15605#diff-7a867d5568f78f326aade88ca1aedea7L185

For example, u can do your own fork, compare v1 and v2 with BC, split all into 1 and viola! Full-pack sharebutton3 release. 🙂


Also, no need to minify js and css files. For they minification anyone may install Hexo additionals plugins like hexo-all-minifier. Source must be open for eyes, right?

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 1, 2017

I'm trying but i meet some troubles when making web font.
It may take some time since I don't have much time now.

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 1, 2017

I'm going to make webfont on fontello like what revir does, but i find that Tapiture,Slashdot,Technorati,Googlebookmarks,Nwsvine,Pingfm,Mailru dont have icons on fontello.
Is it acceptale to remove those social networks?

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 1, 2017

@LEAFERx hi again.

  • Tapiture — no need, was closed in 2015.
  • Slashdot — present from 1997, but i never heard about it.
  • Technoratipresent from 2002, but i never heard about it too. Something like «:mag:» icon.
    Desirable to add it.
  • Googlebookmarks — this service i never used, but i think peoples use it anyway. I think need to add simple any google icon here, but not the same as «G+».
    Desirable to add it.
  • Nwsvine Newsvine — there is typo, this is US community board and have 3k alexa rank.
    Desirable to add it.
  • Pingfm — no need, current status is closed.
  • Mailru (Mail.Ru | Mail Ru Group) — it's like symbol «@», in Ru peoples use it like Douban.
    Need to add it.

U may also see icons here: https://icomoon.io/app/#/select/library


Is it acceptale to remove those social networks?

I think yes, but we must try to add all of possible icons if this options already exists and created.
And remove unused icons too. Well, i think need a little wait for answer from revir's issue and mb report to him about this.

Plugin is really good and i think need to modify him with greatest usability and options.


Bugs

  • I found a little bug: what push on main button — social appear, ok, it's good; and then push on it main button again — social not disappear. Only if u click in any other place of desktop, icons will hide.
  • Please, replace js and css on non-minified files.

@ivan-nginx
Copy link
Collaborator

Hi! Some small bugs found:

  • Titles on social icons when mouse move on them — lowercases. Can we do, for example, instead of
    <span class="need-share-button_link-box ... title="weibo"></span>
    Like this:
    <span class="need-share-button_link-box ... title="Weibo"></span>
    ?

} else {
link.className += " icon-" + network_lc;
}
link.dataset.network = network_lc;

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

if (fontello.indexOf(network_lc) === -1) {
link.className += " social-" + network_lc;
} else {
link.className += " icon-" + network_lc;

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

link.className = iconClass + network_lc;
var fontello = ["weibo", "wechat", "douban", "qqzone", "renren"];
if (fontello.indexOf(network_lc) === -1) {
link.className += " social-" + network_lc;

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

var network_lc = network.toLowerCase();
link.className = iconClass + network_lc;
var fontello = ["weibo", "wechat", "douban", "qqzone", "renren"];
if (fontello.indexOf(network_lc) === -1) {

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

var link = document.createElement("span");
network = myoptions.networks[network].trim();
var network_lc = network.toLowerCase();
link.className = iconClass + network_lc;

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

if (myoptions.networks.hasOwnProperty(network)) {
var link = document.createElement("span");
network = myoptions.networks[network].trim();
var network_lc = network.toLowerCase();

Choose a reason for hiding this comment

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

Identifier 'network_lc' is not in camel case.

@ivan-nginx
Copy link
Collaborator

Identifier 'network_lc' is not in camel case.

network_lc » networkLC

@ivan-nginx
Copy link
Collaborator

So, need to resolve conflicts, test it and can be merged i think.

P.S. Sorry for conflicts, it's my mistake with branches and new gui soft. See last commits in master branch.

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 4, 2017

Sorry but it was the first time i merge such a complex branch
the git auto-merging deletes some lines in some files without noticing me and i really dont know if i left something
can you please resolve the conflicts?

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 4, 2017

Yeah, i just don't understand what's the problem with this merge, all was fine just yesterday.

can you please resolve the conflicts?

image

But i can make another: i can merge your this branch directly in master or dev branch to solve it, but need to do end of the maintenance. Merge ready? All possible bugs and another things fixed? Need to delete all commented lines and make code clean in source/css/_common/components/third-party/needsharebutton.styl, for example. So, please, finish it and i'll try to push this merge directly.

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 4, 2017

I find a strange css bug
the button has different style and hover behavior in four theme scheme
in muse it is black background and white hover background
in mist it is always white background whether hover or not
in pisces&gemini it is white background and black hover background

@ivan-nginx
Copy link
Collaborator

It's not a bug, that styles reused from main buttons NexT styles and on different schemes — different styles. See live demo with buttons here.

Ok, do not edit for now anymore. I just try to resolve this conflicts and merge it into master, afterthat we can resume to fix all bugs, etc.

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 4, 2017

ok

@ivan-nginx
Copy link
Collaborator

@LEAFERx check please button-refactor branch. All NeedMoreShare options there? All latest commits, etc.?

@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 4, 2017

yes, and I think it's ready to merge

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 4, 2017

Ok, i'll do it soon.

P.S. Conflict was, i think, because u or i don not stage pushes into your commit directly. It was my fault, i was must to create separated pull or branch with my own button refactor changes.

No need to create so must difficult commits in next time.

@ivan-nginx ivan-nginx merged commit 583de52 into iissnan:master Oct 4, 2017
@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 5, 2017

Alright, branch was merged. Any additions/features/fixes to this pull are welcome.

Anyway, god job @LEAFERx! Also greatings thanks for Gitment pull and help with adaptation of Gitmint! I really appreciate it.

@LEAFERx LEAFERx deleted the need-more-share2 branch October 5, 2017 03:47
@ivan-nginx
Copy link
Collaborator

About buttons: well, it's inherit styles from main readmore buttons and buttons tags too. Need just redefine in _custom.styl to .btn class any style u want — all buttons (and share button too) will show in this styles.

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 5, 2017

@LEAFERx hi again! How are u? Fine? Ok, there is 2 bugs:

  1. Then user click on any share button and share it, buttons panel after that don't close automatically.
  2. With mobile style icons not flexed in main window:
    image

@LEAFERx LEAFERx restored the need-more-share2 branch October 6, 2017 03:13
@LEAFERx
Copy link
Contributor Author

LEAFERx commented Oct 6, 2017

Sorry that i dont have time to fix it now, my school is begin and a important exam is waiting for me.
No until Nov will i have time i guess.
Sorry again.

@ivan-nginx
Copy link
Collaborator

Ok, no problem, dude. Just take a time and try to fix it later. No need to hurry somewhere, it's opensource.

@online
Copy link

online commented Oct 6, 2017

@ivan-nginx, как по мне, то нужно добавить условие, чтобы NeedMoreShare2 (боковая кнопка, которая прилипает к левой стороне браузера) не выводился на главной странице, а только на страницах заметок блога. Что скажешь?

Живой пример у меня в блоге: https://inotes.pp.ua

@ivan-nginx
Copy link
Collaborator

ivan-nginx commented Oct 6, 2017

@online так в этом и прикол, что он сделал 2 кнопки:

  1. Боковая левая выводится везде.
  2. Кнопка для постов выводится в постах.

И при желании можно включить обе или выбрать какую-то одну. Вообще, интересно он сделал, конечно, я сперва тоже не въехал. Но тут есть над чем подумать. И что пофиксить тоже есть. 😄

Вообще, заметил, что всё же соц-иконы Бирмана выдают цифру репостов некоторых. Скорее всего интегрирую и их тоже + там также через JS, оказывается, сделано (то бишь ссылки индекс и вес страниц забирать не будут).

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

Successfully merging this pull request may close these issues.

4 participants