Closed (fixed)
Project:
Views Infinite Scroll
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
6 Jun 2010 at 11:08 UTC
Updated:
2 Aug 2011 at 19:09 UTC
Jump to comment: Most recent file
Comments
Comment #1
Remon commentedSounds like a good idea :D. I'll take a look and keep this issue updated with any progress.
Comment #2
esteewhy commentedOh 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.
Comment #3
tstoecklerWhy are you against introducing a dependency, esteewhy, could you explain?
Comment #4
esteewhy commentedHey, @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!
Comment #5
tstoeckler-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.
Comment #6
esteewhy commentedYeah, 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.
Comment #7
tstoecklerI think you mean .info file not .install file.
If you introduce a dependency for real, you just put
dependencies[] = librariesin 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.Comment #8
Remon commentedThanks 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.
Comment #9
Remon commentedLibraries-dependency was introduced in latest dev.
Comment #11
nikkubhai commented"Libraries-dependency was introduced in latest dev"
I dont see it. Infact Libraries module and this module are incompatible