Recursive layout rendering depending on the size of the table / screen

antman3351 commented 5 months ago

Hey Oli, I have a problem with Tabulator recursively recalculating the table height when the table has a max-height ( allowing the table to shrink if the data is less than the height )

The really strange thing is it doesn't always happen, I can trigger it by zooming out ( my colleague noticed it when his tab crashed ) so I guess it's dependent on screen resolution / size.

RowManager.redraw() -> RowManager.adjustTableSize() -> this.redraw()

I modified the adjustTableSize to see what's happening and limit it to 100 redraws

if(!this.fixedHeight && initialHeight != this.element.clientHeight){
       if ( this.redrawing )
       {
       this.redrawingCount++;
       }
       console.log( `initialHeight: ${initialHeight} this.element.clientHeight: ${this.element.clientHeight}. isRedrawing:${this.redrawing ?1 : 0 }`);

      resized = true;
      if(this.subscribed("table-resize")){
          this.dispatch("table-resize");
      }else if(this.redrawingCount < 100){
          this.redraw();
      }
}

Output when zoomed:

initialHeight: 20 this.element.clientHeight: 749. isRedrawing:0 [RowManager.js:1076:27](http://localhost:8082/totalgest/4.0.0/public/assets/global/plugins/tabulator/src/js/core/RowManager.js)
initialHeight: 725 this.element.clientHeight: 1045. isRedrawing:1 [RowManager.js:1076:27](http://localhost:8082/totalgest/4.0.0/public/assets/global/plugins/tabulator/src/js/core/RowManager.js)
initialHeight: 1045 this.element.clientHeight: 749. isRedrawing:1 [RowManager.js:1076:27](http://localhost:8082/totalgest/4.0.0/public/assets/global/plugins/tabulator/src/js/core/RowManager.js)
initialHeight: 725 this.element.clientHeight: 1045. isRedrawing:1 [RowManager.js:1076:27](http://localhost:8082/totalgest/4.0.0/public/assets/global/plugins/tabulator/src/js/core/RowManager.js)
initialHeight: 1045 this.element.clientHeight: 749. isRedrawing:1 [RowManager.js:1076:27]
...

As a fix I've just left the flag to not call this.redraw() when being called from redraw(), I haven't noticed any visual differences so far. I've waited to make a PR because I'd like your opinion. If the problem is caused else where maybe we can still leave a max recursion check and then throw an error ?

Also Is this line correct in VirtualDomVertical.js@101 ?

if(this.rows().length){
    this._virtualRenderFill((topRow === false ? this.rows.length - 1 : topRow), true, topOffset || 0);
}else{

in the if should this.rows.length be a function call this.rows().length as a quick console.log( console.log( this.rows.length, this.rows().length ); in the if gives 0 124

changing the inside call to this.rows() doesn't fix the infinite loop issue.

Tabulator Info

6.2.0 ( also in 6.0.1 )

To Reproduce I can't reproduce the exact error in JS Fiddle but the browser does hang ( chrome ) and ask if I want to interrupt the script ( no idea if it's the same problem or another issue ) https://jsfiddle.net/antman3351/c1asqtez/53/ make sure the console is minimised ( it doesn't do it with the console open )

Desktop (please complete the following information):

OS: Windows 10
chrome, safari

Additional context Add any other context about the problem here.

olifolkerd commented 5 months ago

Is your table element or it's parent flexed?

antman3351 commented 5 months ago

Hey,

No the table only has a maxHeight passed in the table options

I recreated a simple version of HTML/JS outside of jsFiddle and put it in a html file

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <link href="https://unpkg.com/tabulator-tables@6.2.0/dist/css/tabulator.min.css" rel="stylesheet">
</head>
<body>
    <div id="example-table"></div>
    <script>
        (function ()
        {
            this.init = async () =>
            {
                let TabulatorUrl = null;
                if ( confirm( 'Use patched version ? ' ) )
                {
                    console.log( 'Using patched version');
                    TabulatorUrl = 'http://localhost:8082/totalgest/4.0.0/public/assets/global/plugins/tabulator/6.2.0/js/tabulator_esm.custom.js';
                }
                else
                {
                    console.log( 'Using official build');
                    TabulatorUrl = 'https://unpkg.com/tabulator-tables@6.2.0/dist/js/tabulator_esm.js';
                }

                const { TabulatorFull } = await import( TabulatorUrl );

                //define some sample data
                const _tableData = [
                    { id: 1, name: "Oli Bob", age: "12", col: "red", dob: "" },
                    { id: 2, name: "Mary May", age: "1", col: "blue", dob: "14/05/1982" },
                    { id: 3, name: "Christine Lobowski", age: "42", col: "green", dob: "22/05/1982" },
                    { id: 4, name: "Brendon Philips", age: "125", col: "orange", dob: "01/08/1980" },
                    { id: 5, name: "Margret Marmajuke", age: "16", col: "yellow", dob: "31/01/1999" },
                ];

                let tableData = [];
                for ( let i = 0; i < 10; i++ )
                {
                    tableData = [ ...tableData, ..._tableData ];
                }

                const table = new TabulatorFull( "#example-table", {
                    maxHeight: '90vh',
                    data: tableData,
                    layout: "fitDataStretch",
                    autoResize: false,
                    columns: [ //Define Table Columns
                        { title: "Name", field: "name" },
                        { title: "Age", field: "age", hozAlign: "left" },
                        { title: "Favourite Color", field: "col" },
                        { title: "Date Of Birth", field: "dob", hozAlign: "center" },
                    ],
                } );

                table.on( 'tableBuilt', () =>
                {
                    console.log( 'table built' );
                    // table.blockRedraw(); // Makes no difference
                } );
            };

            this.init();

        }).apply( {} );

    </script>
</body>
</html>

Here's what happens ( the patched version doesn't allow calling the redraw function if adjustTableSize is already called from redraw )

https://github.com/olifolkerd/tabulator/assets/16060462/022c69f4-1341-4b21-b175-ad6b720a107c

antman3351 commented 5 months ago

Update: I had to also skip re-dispatching the table-resize event too as I had a table with the total column widths > table width ( horizontal scrollbar ) that too kept recursing over the same section.

Change now looks like this:

if(!this.fixedHeight && initialHeight != this.element.clientHeight){
  resized = true;
  if(!this.redrawing) // Recursion check
  {
      if ( this.subscribed( "table-resize" ) )
      {
          this.dispatch( "table-resize" );
      }
      else
      {
          this.redraw();
      }
  }
}

timhemming-fico commented 3 months ago

We're also hitting this same issue, the table is recursively redrawing. I've applied the same fix to a patched version of Tabulator and it resolves the issue:

GolfSierra commented 2 months ago

Same issue here, but the fixes do not work for me

ergsolo commented 2 months ago

I got stucked tabulator on small amount of data and then an Error

In my case looks like dispatch('resize-table') is called recursively

Tabulator 6.2.0

ergsolo commented 2 months ago

I've downgraded tabulator to 5.5.4 and it works without issues. Issue starts producible since 5.6.1.

BartNSTCL commented 1 month ago

I've had this hit at least 1 user, maybe 2 out of over a hundred. It was odd, because the pages worked in Edge but not Chrome. I went and checked and the user's Chrome was set to 80% zoom. Based on the comments above, maybe if he was running at 100%, the error wouldn't occur. For now, I've rolled back to 5.5.4 as suggested earlier.

BartNSTCL commented 3 weeks ago

Has this been addressed by version 6.2.5?

olifolkerd / tabulator

Recursive layout rendering depending on the size of the table / screen #4467