DataTables / Responsive

Responsive extension for DataTables, providing support for complex tables on all device screen sizes
Other
148 stars 86 forks source link

Endless loop with `_redrawChildren` in Responsive 2.4.0 #125

Closed LuigiPulcini closed 9 months ago

LuigiPulcini commented 1 year ago

A change in Datatables Responsive 2.4.0 seems to be causing an issue when a third-party script updates the content of a cell inside a hidden row.

https://github.com/DataTables/Responsive/commit/57ec443d28408163d85d1c858940b109392e5f0e#diff-436c94a81df16d4b1d48e10ec42c50841c5323996a148c1bf937f08353562b23L863-R925

We have a plugin that, among other things, adds a WooCommerce variation form inside a cell of the table. When the _redrawChildren method was moved outside of the if (changed) {} conditional clause in version 2.4.0, we started experiencing an endless loop between Datatables and WooCommerce: essentially, every time the _redrawChildren gets called, WooCommerce updates the content of the variation form triggering another call of the _redrawChildren. Hence, the endless loop.

If the _redrawChildren call is placed back inside the conditional clause, as it was in version 2.3.0, without any other changes to the responsive extension, the endless loop stops occurring.

I was wondering whether _redrawChildren should be put back inside the if (changed) {} conditional clause or moved before changed is calculated.

AllanJard commented 1 year ago

Sorry for the delay in getting back to you here.

I've just made a couple of changes to Responsive that might effect this. Could you possibly try the latest from this repo? If it doesn't resolve it, can you give me a link to a test case showing the issue please?

The reason the _redrawChildren function was pulled out of the changed check was for the listHiddenNodes display option which needs it to always run.

LuigiPulcini commented 1 year ago

Hi, @AllanJard, thank you for looking into this.

I can confirm that the problem is still there when using Responsive 2.4.1 or the master branch from this repository.

This is how to reproduce the issue:

  1. go to https://plugintesting.barn2.com/producttable/product-table-test/product-table-filters/
  2. switch to a mobile screen size (anything narrower than 460px in width would do)
  3. scroll down to a product called "Cap" (which is a variable product)
  4. now click the "+" plus button to open the child row
  5. you will see the variation form relative to the current product

At this point the issue could be exposed either visually or functionally:

We will try to keep the testing website at this version for as long as you need to check it out.

Thanks again!

AllanJard commented 9 months ago

Hi,

I've just committed a change to move the _redrawChildren call back into the condition. It has involved updating how the cloned table for size calculations is constructed.

I should note that this is on the dt2 branch for this repo (to be released as Responsive 3) and depends upon DataTables 2, which I'm hoping to ship before the year end (or possibly early January). Responsive uses a number of the new APIs for multi-row header support. It probably would be possible for one to back port this change to Responsive 2 if you need the change before DT2 ships.

Sorry it has taken me so long to sort this one out.