FOSCommentBundle icon indicating copy to clipboard operation
FOSCommentBundle copied to clipboard

Javascript is outdated and needs an overhaul

Open nreynis opened this issue 8 years ago • 4 comments

The javascript client has several problems and needs a bit of attention:

  • ~~Use of deprecated methods which have been removed from jQuery 3 (critical: jQuery 3 has been released 18 months ago! Any new project is likely to use it and will have problem with this bundle.)~~
  • Use of assetic which is deprecated. You should deliver something more webpack friendly as webpack/remix is now the recommanded tool for Symfony
  • Global variables. You should use html5 data-attributes instead
  • Auto initialisation on parse. You should init on DOMContentReady so that the user is free to put his script where he wants

nreynis avatar Jan 16 '18 09:01 nreynis

Just found #604. Forcing dev stability solve the first issue composer require friendsofsymfony/comment-bundle:^3.0@dev.

I still thinks the other points are valid and should be addressed in some way. But that's more of an opinion, so feel free to close and ignore if you don't agree.

nreynis avatar Jan 16 '18 10:01 nreynis

for the global variables vs data attributes, a PR adding support for data-attributes would be OK (but not removing support for the global variable, to prevent BC breaks).

Note that I don't know of any FOS member actively using this bundle anymore (I may be wrong about this though), so changes are relying on community contributions.

stof avatar Jan 16 '18 10:01 stof

@stof could you share what people are using as an alternative? Perhaps it is time to concentrate resources into a better package if available and formally deprecate/abandon this one.

yakobe avatar Jan 19 '18 11:01 yakobe

jQuery 3 is supported in master branch. I'll make a new release as soon as the next FOSRestBundle is released.

I agree it would be nice to have HTML 5 data attributes, perhaps someone is willing to create a PR?

I also agree on DOMContentReady. That should be an easy fix.

Regarding the assets, unfortunately I have not yet used Webpack so I have no clue what needs to be changed.

XWB avatar Jan 29 '18 09:01 XWB