Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
antman3351 opened this issue Apr 17, 2024 · 3 comments
Open
Labels
Possible Bug A possible bug that needs investigation

Comments

@antman3351
Copy link
Contributor

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.

image

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.

@antman3351 antman3351 added the Possible Bug A possible bug that needs investigation label Apr 17, 2024
@olifolkerd
Copy link
Owner

olifolkerd commented Apr 17, 2024

Is your table element or it's parent flexed?

@antman3351
Copy link
Contributor Author

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/[email protected]/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/[email protected]/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 )

tabulator.recursion.mp4

@antman3351
Copy link
Contributor Author

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();
	  }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Possible Bug A possible bug that needs investigation
Projects
None yet
Development

No branches or pull requests

2 participants