Libraries API is a generic handler of external libraries.
This module relies on an external library but users need to place it inside the modules directory, which becomes a problem, when files change between releases, forcing users to delete the module directory in which case their external library is gone. With Libraries API users can place all their libraries in a sites/all/libraries directory.
Since your module is pretty new, it might make sense, to do this as quickly as possible, as making the change once a bunch of users have installed your module might be painful.

Comments

Remon’s picture

Sounds like a good idea :D. I'll take a look and keep this issue updated with any progress.

esteewhy’s picture

Status: Active » Needs review
StatusFileSize
new1.5 KB

Oh no, please no dependencies, but let the user select :-)

I've done a patch. Let me give the following remark: this patch is coded in such a way that when we have enabled libraries.module, then we must place jquery-autopager in 'libraries' directory, i.e.: there's no fall-back checking. I think it's okay to enforce a good programming habits. Still, this behaviour should probably be mentioned in the README.txt

Another remark: this patch also contains a fix for #821184: Selecting more specific content with Table style.

tstoeckler’s picture

Why are you against introducing a dependency, esteewhy, could you explain?

esteewhy’s picture

Hey, @tstoeckler, I'm against the "dependency" in a sense that libraries.module must not be a mandatory requirement for views_infinite_scroll.module.

Instead, I'd prefer that views_infinite_scroll.module could take advantage of it when it's available and fall back to existing logic if it is not.

That's exactly what my patch does - go try yourself and, please, share your impressions!

tstoeckler’s picture

-Yes, I've read your patch, and I get what it does.
No I don't have any hard feelings about this, but I'll just make my point for the sake of the argument.
In my opinion there is just no point in being able to put the library in the module folder. It is a bad pattern for a number of reasons, a few elaborated at the top of the post. Also, in the future Libraries API will provide even easier loading of libraries, as well as centralized Installation instructions. Crazy future plans might include automatic download methods, etc. All I'm trying to say is that if you introduce the dependency on Libraries API now, when you have nothing to lose, in that you don't have to provide for an upgrade path yet (which might be tedious, because introducing a new dependency as part of an update doesn't always go well in Drupal), you gain a little now (in that your users can put the library in site-specific folders) and you win a lot in the future.

Regarding your patch: instead of the module_exists('libraries'), you might just want to do function_exists('libraries_get_path') as that's faster.

esteewhy’s picture

Yeah, I'm with You that libraries.module is a chance to enforce a good habits.

As to performance, I'd even go that far as to push dependency checks into .install file - i've even saw some modules doing just that and posting warning to Drupal status page.

tstoeckler’s picture

I think you mean .info file not .install file.
If you introduce a dependency for real, you just put dependencies[] = libraries in the .info file. The problem with that is that users, who already have that module installed but do not have Libraries API, will get a WSOD on every page that loads a library, potentially every page, so that there is no way out. With a new module without a stable release or many users, you don't have those problems.

Remon’s picture

Status: Needs review » Postponed (maintainer needs more info)

Thanks for your enlightening discussion :), but I might drop dependency on js plugins altogether. Check this #714766: Alter drupal.Views.Ajax.ajaxViewResponse js function to enable endless scrolling.

Remon’s picture

Status: Postponed (maintainer needs more info) » Fixed

Libraries-dependency was introduced in latest dev.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nikkubhai’s picture

"Libraries-dependency was introduced in latest dev"

I dont see it. Infact Libraries module and this module are incompatible