Replace ajax loads with inline html widgets. Improve load time by 90%+

bengtan - September 16, 2009 - 04:02
Project:editablefields
Version:6.x-2.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed
Description

Hi,

I'm developing a site that uses editablefields. I have a view that displays a list of nodes with a number of editable fields.

It's all working as expected ... except that the view takes a long time to load because it's making a separate ajax request for each field of each node. So ie. if I have 5 nodes with 4 editable fields each, that's 20 extra requests.

This starts to get very very slow :)

I have a patch that fixes this by getting rid of the ajax calls, and instead, loads the field widgets via inline html. This results in a vast improvement in load time.

This patch adds a new cck formatter 'Editable (HTML)' (and renames the old formatter to 'Editable (Ajax)'). The site administrator selects the appropriate formatter depending on which behaviour they desire.

Arguably, the 'Editable (HTML)' formatter doesn't really work for non-javascript-enabled browsers, but I think this is a suitable trade-off that the end user can decide on for themselves.

So, here's the patch. It's against editablefields 6.x-2.x-dev as of 2009 Sept 15th.

Please have a look and let me know if there are any questions, comments, feedback, modifications required etc.

Next thing I have to deal with is #488816 and I'll probably do a patch for that. So, if this patch can be merged into the module sooner rather later, that would be very helpful so I/we don't have to maintain two separate mergesets.

Thank you.

AttachmentSize
editablefields-html-load.patch.txt5.14 KB

#1

bengtan - September 16, 2009 - 05:21

Since I was also working on #488816, here is a cumulative patch for #488816 and this issue. Again, diff'ed against editablefields 6.x-2.x-dev as of 2009 Sept 15th.

AttachmentSize
editablefields-html-load.578678-2.patch.txt 5.17 KB

#2

markfoodyburton - September 16, 2009 - 06:17

I think this is a good idea. Somewhere there should be a mention that this will cause non-JS enabled browsers to fail (which, you are right, is why I initially decided to do this all in JS) - but - how many of those browsers are out there now? - fewer and fewer I guess...

So - I think this is a great plan.

Bengtan, I have granted you CVS access, it will make it considerably easier for you to check this in (and - fix some other bugs while your about it :-) )

Cheers

Mark.

#3

bengtan - September 16, 2009 - 07:51

HI Mark,

Thanks for that.

Editablefields doesn't work without javascript anyway, so I see the breakage with this patch as minor. If the breakage is a concern, the user can always opt to use ajax loads.

So... is this the green light for me to commit the patch?

The reason I ask is that I've only looked at the bits that are relevant to me, and there is a small chance I may have broken something accidentally. Hence, it'd be nice for someone else knowledgeable (ie. a maintainer) to have a quick look at it.

#4

markfoodyburton - September 16, 2009 - 07:58

Count yourself as a maintainer :-)
My personally feeling,
Commit - ask people to test from the CVS,- make a release if nobody shouts :-)))

For my part - I'm not likely to be able to check this for a while :-(

Cheers

Mark.

#5

bengtan - September 17, 2009 - 07:53
Status:needs review» fixed

Patch from comment 1 has been committed to cvs. This also incorporates a fix for #488816: Requires "Edit Any" Permission.

#6

bfr - September 30, 2009 - 14:16

So, does this remove the need for ajax load module? It brakes views display tabs which prevents me using this great module.

#7

bengtan - October 1, 2009 - 02:25

@6: I'm not sure. It probably depends on what sort of fields you are making editable. I'd suggest you test on your own site.

#8

bfr - October 1, 2009 - 13:09

How do i edit the module so that i can disable ajax module from admin/build/modules? Now it is marked as "required" and cannot be disabled without disabling editablefields.

edit: Do i brake something if i disable it straight with sql query? Or will drupal still automatically disable editablefields when it notices that ajax_load is disabled from database?

#9

bengtan - October 1, 2009 - 15:41

Aiks. I didn't realise editablefields is dependent on ajax load. In that case, no, don't disable ajax load.

#10

bfr - October 2, 2009 - 11:13

Would you then have any clue why the ajax_load brakes views display tabs? I know i'ts not the fault of you or the developer of excellent editable fields module, but i dont get response
from views display tabs issue queue, and i'm getting desperate since i would really need both modules..(i can so some things with editviews but this is just so much better..)

#11

bfr - October 8, 2009 - 13:49

How do you save the changes for the edits when html is enabled?

#12

bengtan - October 9, 2009 - 02:19

@11: The saving is still done through ajax.

#13

alexevasion - October 21, 2009 - 18:59

This is great! However, I find that I get an "Error, unable to make update:" message every time I update... even though it works. Also, attempting to trim the node field in the view settings breaks the module... then nothing is displayed in the editable field column.

#14

System Message - November 4, 2009 - 19:00
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.