Skip to content

[3.0] Port to vanilla Javacript, without jQuery or other dependencies - #22

Merged
richard67 merged 33 commits into
richard67:3.0-devfrom
onli:develop
Dec 29, 2025
Merged

[3.0] Port to vanilla Javacript, without jQuery or other dependencies#22
richard67 merged 33 commits into
richard67:3.0-devfrom
onli:develop

Conversation

@onli

@onli onli commented Sep 20, 2023

Copy link
Copy Markdown

This takes the code from the develop branches and moves it to modern JS: Regular modules, no jQuery, no url require module. To create a shariff.complete.js (for the demopage) I used rollup.js, like this:

rollup src/js/main.js -o demo/shariff.complete.js

It's likely there are alternatives. The javscript files with the modern modules also work directly in the browser, if you embed the main.js.

The facebooklike functionality is kept, but wasn't testable for me. I did not understand the purpose of the facebooklike_dlg.js with a reference to .less file (?), so I could not re-add it when restoring the dialog functionality. You might have to fix some aspects there.

In general, the whole code was only tested in the browser, without compiling the CSS.

The CSS side of the project wasn't touched. This PR does not contain the changes to the package.json or other documentation files.

@richard67

Copy link
Copy Markdown
Owner

Thanks. Will check on weekend.

@onli

onli commented Sep 20, 2023

Copy link
Copy Markdown
Author

Cat pressed the enter button, I edited in a description now.

@bmarwell

Copy link
Copy Markdown

@richard67 I am still using shariff-plus. Can you merge an publish this? I will happily create an X button (to replace Twitter) and maybe create a new backend. The recent ones do not really work, it seems.

@richard67

Copy link
Copy Markdown
Owner

@richard67 I am still using shariff-plus. Can you merge an publish this? I will happily create an X button (to replace Twitter)

@bmarwell This PR has merge conflicts, and the change to vanilla JS is not complete. The build process is broken with this PR.

For an X button it doesn't need this PR, I have something ready here which us currently being tested, and when I find the time I will create a new release with that.

and maybe create a new backend. The recent ones do not really work, it seems.

Which backend do you mean? If you are using https://github.com/heiseonline/shariff-backend-php , it might be that it doesn't work on new PHP versions. But my fork https://github.com/richard67/shariff-backend-php works fine for me on my homepage with PHP 8.3.

@bmarwell

Copy link
Copy Markdown

Thanks, will try that!

@onli

onli commented Apr 17, 2025

Copy link
Copy Markdown
Author

Hi @richard67

Just to note, I don't remember the change as incomplete, and am definite the build process was only changed and not broken - which makes sense, since it's browser native JS now the JS side does not need the old build process anymore. The merge conflicts are likely just the flattr removal and upstream service additions, should be easy to fix. Let me know if it would be of help if I try to resolve them.

However, the plugin for which I used shariff now works without it, so there is no need on my side anymore to get this merged. This PR is completely a gift to you if you want the reduced complexity. If not, that's fine.

@bmarwell

Copy link
Copy Markdown

Sounds great. I don't want to open yet another fork, so any progress here with upload to npm is highly appreciated. 😀

I'm a Java developer, but I can try to contribute a little. I'll be happy to dig into it.

@richard67

richard67 commented Apr 18, 2025

Copy link
Copy Markdown
Owner

Just to note, I don't remember the change as incomplete, and am definite the build process was only changed and not broken - which makes sense, since it's browser native JS now the JS side does not need the old build process anymore.

@onli Sorry, I was tired last night and maybe wrote my answer to quickly, so I used the wrong wording. And maybe it's just me not fully understanding your changes as I'm not a JS guy.

The existing implementation without this PR allows to run npm run build to compile the less files into one single css and the js files into one single js, and then by running npm run build_zip to create the zip file for the release, and people could use it in the same way as the upstream heiseonline/shariff. And you could run npm run dev to create and then show the demo in a local browser. It can be seen in the package-json and the .travis.yml file, which is not used anymore because travis is dead, but still can serve as some kind of documentation.

Does all that still work with your PR? I am not sure, and I haven't tried that yet. I've never used rollup.js and I don't know how common it is. If it works, it should at least be added to the npm dependencies, and the comands in package.json should be updated with that.

Another thing is that I don't know yet which way I want to go with shariff-plus, and that depends on if the upstream heiseonline/shariff is really abandoned or still alive somehow.

Up to now, I kept my shariff-plus and its code compatible with heiseonline/shariff. The only difference was my additional Facebook Like button (which doesn't work anymore in parts of the world where they haver something like GDPR so Facebook disabled it for these countries), and maybe I had merged the one or other PR from the upstream repo which was not merged there yet but very likely would be merged later.

In this way I was still able to update my fork to the changes from upstream if they finally got merged there.

This would be harder with your PR merged.

But as the upstream repo seems to be close to death, if not dead already, the question becomes more urgent which was I should go here, stick with the old and partly unnecessarily complicated stuff inherited from the upstream shariff, or decouple shariff-plus from upstream and get rid of that old stuff.

For the latter your PR could indeed a huge step into the right direction.

Up to now I did not want to make that decision because I still had hope that something happens upstream so I could pull that into my fork. I have already pulled in the changes from PRs which were merged upstream but not released yet and the changes from the open PRs heiseonline#434 and heiseonline#444 .

See 2.3.1...develop

That's where the conflicts come from.

The merge conflicts are likely just the flattr removal and upstream service additions, should be easy to fix.

I agree, the merge conflicts are not a big problem.

Let me know if it would be of help if I try to resolve them.

That's much appreciated, especially since you don't need shariff-plus anymore for your plugin.

I think I will do following:

  • Make soon a new release 2.4.0 2.3.2 with the merged changes but without this PR.
  • Create a 3.0-dev branch and rebase your PR to that branch (and solve conflicts).

In this way I still can keep by develop branch in synch with the upstream, and in parallel we can work on the development of a version 3 , e.g. finalise the build process with rollup.js or similar, and clean up dependencies by things from the old build process which are not needed anymore, like e.g. webpack.

Would be nice of course if you or @bmarwell could help with that.

@richard67

Copy link
Copy Markdown
Owner

I've just published a new release 2.3.2, see https://github.com/richard67/shariff-plus/releases/tag/2.3.2 . I've just published to NPM, too.

@richard67

richard67 commented Apr 18, 2025

Copy link
Copy Markdown
Owner

@onli I've updated your branch and solved the merge conflict, which was due to the flattr removal, and have rebased your PR to the newly created 3.0-dev branch.

@richard67
richard67 changed the base branch from develop to 3.0-dev April 18, 2025 13:41
@richard67 richard67 changed the title Port to vanilla Javacript, without jQuery or other dependencies [3.0] Port to vanilla Javacript, without jQuery or other dependencies Apr 18, 2025
@richard67

Copy link
Copy Markdown
Owner

@bmarwell For the backend see https://github.com/richard67/shariff-backend-php/releases/tag/10.1.0 . Could you check and report back if that works for you?

@bmarwell

Copy link
Copy Markdown

@bmarwell For the backend see https://github.com/richard67/shariff-backend-php/releases/tag/10.1.0 . Could you check and report back if that works for you?

Yup, works:

$ curl -L 'https://blog.bmarwell.de/shariff/index.php?url=https%3A%2F%2Fblog.bmarwell.de%2F2025%2F04%2F12%2Fcreating-app-distribution-using-maven-jreleaser.html'
{"stumbleupon":0,"pinterest":0,"xing":0,"buffer":0,"vk":0}%

@bmarwell

Copy link
Copy Markdown

Hmm I just saw that I use only one of the supported services (I have enabled: X/Twitter, Whatsapp, LinkedIn, Facebook).

@richard67

Copy link
Copy Markdown
Owner

Hmm I just saw that I use only one of the supported services (I have enabled: X/Twitter, Whatsapp, LinkedIn, Facebook).

@bmarwell You mean for the backend? Twitter stopped their sharing counters 10 years ago, see heiseonline/shariff-backend-php#48 . LinkedIn was removed in 2019, see heiseonline/shariff-backend-php#159 . WhatsApp I don't find now in the history, possibly it never provided sharing counters. It seems your backend is or was very outdated.

@richard67

Copy link
Copy Markdown
Owner

@bmarwell P.S.: You can see the up to date demo here: https://www.richard-fath.de/shariff-plus-demo/index.html

@bmarwell

Copy link
Copy Markdown

Hmm. The backend shows a number, but I don't see the number displayed.

https://blog.bmarwell.de/2025/04/12/creating-app-distribution-using-maven-jreleaser.html

@bmarwell

Copy link
Copy Markdown

Oh, it's facebooklike. Hadn't seen that on the readme. Sorry.

Comment thread src/js/shariff.js Outdated
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@richard67

Copy link
Copy Markdown
Owner

But the state now should really be a good starting point for future development.

@onli Yes, you did an amazing job. Thanks a lot.

Regarding Windows: As the build commands in the package.json contain Unix shell commands (sh without bash extensions), it requires WSL to run them on Windows. So symbolic links would not be an additional problem. But I'm not sure yet if I want to keep that or change it so it would work also on Windows without WSL.

Regarding minified js and css: Can you recommend a minifier?

Regarding copying the webfonts to dist: That was done by webpack, as far as I understand it. Now it can be done with a simple copy command. I will add the necessary changes to this PR before merging it.

@richard67

Copy link
Copy Markdown
Owner

@onli I've allowed myself to commit some changes to this PR so at the end the behaviour of the build and the demo are the same as before, e.g. the fonts are copied to the dist and the demo, and the symbolic links are not needed. I've done it in separate commit where the commit message describes what it does so you can understand my changes.

@richard67

Copy link
Copy Markdown
Owner

P.S.: So the only thing which is missing is the minification of the js and css files.

@onli

onli commented May 4, 2025

Copy link
Copy Markdown
Author

Thanks @richard67, I'm happy this works for you. For the minifier, maybe https://www.npmjs.com/package/minify would work?

@bmarwell

Copy link
Copy Markdown

... please merge this....

@richard67

Copy link
Copy Markdown
Owner

... please merge this....

Sooner or later I will, but at the moment I have other priorities.

Check my activities in my GitHub profile and you will see what they are.

Bothering volunteer contributors to free open source with such demands is not really helpful.

Feel free to create a fork of this repository and do the work yourself if I am not fast enough, or look for a commercial product of your needs.

@bmarwell

Copy link
Copy Markdown

Sorry, didn't mean to push. But since this library is very popular and has quite a user base, this also means people will be kindly requesting features and - like in this case - even create PRs. However, if you cannot manage the library on your own time alone, may I kindly suggest adding contributors/maintainers to the repository? I honestly that would serve the community better than just creating yet another fork.

But that's merely a suggestion. Perhaps that's an option for you, maybe not.

Anyway, thanks for being a maintainer and responding to my comment (and sorry for the brevity).

@richard67

Copy link
Copy Markdown
Owner

But since this library is very popular and has quite a user base, ...

@bmarwell Sure you are not mixing it up with the original Shariff https://github.com/heiseonline/shariff / https://www.npmjs.com/package/shariff ?
I don't see a big number of downloads for my fork here on https://api.github.com/repos/richard67/shariff-plus/releases or https://www.npmjs.com/package/shariff-plus .

I will handle this PR here when I find the time.

Comment thread src/js/shariff.js Outdated
Comment thread src/js/shariff.js Outdated
- Clone the Default object with `Object.assign` so it's not modified
- CS remove redundant new line with spaces.
@richard67

Copy link
Copy Markdown
Owner

@onli I've allowed myself to apply my latest 2 code change suggestions.

  1. When running the demo I saw that wrong button style "icon" was used where no button style was given with a data attribute.
    The reason was that the Defaults object was assigned by reference and so modified somehow.
    I have fixed that by using Object.assign to clone that object to the options.

  2. There was a new line with some spaces which I've removed.

I will merge this PR soon into the 3.0-dev branch and make further changes for that branch before creating a new major release 3.0.0.

If something is wrong with my changes mentioned above, let me know here or make a new PR to the 3.0-dev branch for fixing it.

Thanks for your work on this PR here.

@richard67
richard67 merged commit a4fce70 into richard67:3.0-dev Dec 29, 2025
@bmarwell

Copy link
Copy Markdown

Woohooo! Great! Can't wait for it! Big thanks! 😊

@onli

onli commented Dec 29, 2025

Copy link
Copy Markdown
Author

Thanks as well :)

@richard67

richard67 commented Dec 29, 2025

Copy link
Copy Markdown
Owner

@onli Meanwhile I have found an issue which I've fixed in my new PR #26 , together with dependency updates and minifying JS and CSS.

The PR is work in progress.

There are still two issues remaining, maybe you can have a look?

You can see them when comparing the demo of version 2 with the demo of my PR which includes also your merged PR.

The issues:

  • When both services facebooklike and facebook are provided via the data-services attribute in exactly the mentioned order, the facebook button is shown before the facebooklike button, i.e. for these 2 buttons the ordering from the data-services attribute is not respected.
    See 5th, 6th and 7th example of the demo.
  • The CSS for the facebooklike dialog is not loaded.

Would be cool if you can check especially for the first issue and report back in my PR.

Thanks in advance.

Update 2025-12-30: The first mentioned issue I have fixed in my new PR.

Update 2025-12-30 (2): The second mentioned issue I have meanwhile fixed in my new PR, too.

@richard67

Copy link
Copy Markdown
Owner

@onli @bmarwell New release created, see https://github.com/richard67/shariff-plus/releases/tag/3.0.0 . I've also published it in NPM.

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