search

NikolayRys / Likely

The social sharing buttons that aren’t shabby
ISC License
394 stars 61 forks source link

Disable requests if no counters #170

Closed vitkarpov closed 4 years ago

vitkarpov commented 4 years ago

This PR deals with https://github.com/NikolayRys/Likely/issues/145

If data-counters is set to an empty string, Likely still makes requests for counters even they're not being used! (Pls, check out https://codepen.io/iamakulov/pen/VQvqem)

The problem is fixed now. Also, I added data-counters to Readme.

vitkarpov commented 4 years ago

Thanks for the review. I'll take a look, didn't know the logic exists. Anyway, the test is there so it's easy to check now.

vitkarpov commented 4 years ago

@NikolayRys I supported "no" for data-counters. However, changes that I've already made are valid and essential as well (and the test shows that). I'll leave comments near the code itself to explain what problem it addresses.

NikolayRys commented 4 years ago

Thanks for a good job and taking care of this. It's mergeable now. Could you please run build to resolve the conflicts?

vitkarpov commented 4 years ago

@NikolayRys done, rebased and built. Look, just noticed, where are the CI checks? 🤔 Did you turn them off?

NikolayRys commented 4 years ago

Thanks! I'm generally not in favor of squashing, but I also don't mind if happening. If you will be submitting anything else in the future, don't bother with it squashing, it's helpful to keep the history :)

Good call about the builds. No, as long as I am in the project, I was assuming we didn't have them at all, but you're right, we have a Travis file here indeed. We'll need to bring it back, I'll create a ticket.

vitkarpov commented 4 years ago

Gotcha 👍