olifolkerd / tabulator

Interactive Tables and Data Grids for JavaScript
http://tabulator.info
MIT License
6.71k stars 818 forks source link

Horizontal Scroll Position Lost on Header Filter, Header Sort or Pagination Changes (only with Remote Pagination, Filter & Sort) #4494

Open Anton-Plagemann opened 5 months ago

Anton-Plagemann commented 5 months ago

Describe the bug Reopening #3450 as a new issue because the old one has been closed with a partial fix only. The issue: The horizontal scrolling position gets lost when using header filters, header sorting, changing page size or changing page AND remote pagination/filtering/sorting is enabled. Additional details can be found in #3450.

Tabulator Info

Working Example Could not provide a working example with jsfiddle because a server is needed for remote pagination/sorting/filtering.

To Reproduce

  1. Create a table with remote pagination, filtering & sorting, e.g.

    new Tabulator(tableHTMLElement, {
    pagination: true,
    paginationMode: "remote",
    filterMode: "remote",
    sortMode: "remote",
    ajaxURL: `https://example.com/getTableData`,
    columns: [
      {
        title: "Some column (repeat until a horizontal scrollbar appears)",
        field: "someRemoteField",
        headerFilter: "input",
      },
    ],
    });
  2. Scroll to the right of the table.

  3. Click on a column header (sorting), enter a value into the header filter input, or switch to page 2.

  4. See the horizontal scrolling position reset to the most left position.

Expected behavior The scroll position should not reset.

Desktop:

Additional Info I want to express my appreciation for the Tabulator package and the continuous efforts to improve it. The functionality it provides has been instrumental in developing complex but intuitive interfaces. Thank you for your dedication and hard work on maintaining such a valuable tool ❀️

Anton-Plagemann commented 5 months ago

I noticed, that this behaviour (horizontal scroll bar reset) seems to be fixed with the following four small modifications:

Original: https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/core/CoreFeature.js#L11-L13

Changed:

// Added replace argument
reloadData(data, silent, columnsChanged, replace = false) {
    return this.table.dataLoader.load(data, undefined, undefined, replace, silent, columnsChanged);
}

Original: https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Sort/Sort.js#L234-L244

Changed

refreshSort(){
    if(this.table.options.sortMode === "remote"){
        this.reloadData(null, false, false, true);  // Added last argument
    }else{
        this.refreshData(true);
    }

    //TODO - Persist left position of row manager
    // left = this.scrollLeft;
    // this.scrollHorizontal(left);
}

Original: https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Filter/Filter.js#L571-L583

Changed:

refreshFilter(){
    if(this.tableInitialized){
        if(this.table.options.filterMode === "remote"){
            this.reloadData(null, false, false, true); // Added last argument
        }else{
            this.refreshData(true);
        }
    }

    //TODO - Persist left position of row manager
    // left = this.scrollLeft;
    // this.scrollHorizontal(left);
}

Original: https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Page/Page.js#L773-L802

Changed:

case "remote":
    this.dataChanging = true;
    return this.reloadData(null, undefined, undefined, true)  // Added the last three arguments
        .finally(() => {
            this.dataChanging = false;
        });

I do however not know if these changes would break something else. @olifolkerd: If you think those changes would be ok, I would be happy to submit a PR 😊

Ariharan2002 commented 4 months ago

I noticed, that this behaviour (horizontal scroll bar reset) seems to be fixed with the following four small modifications:

Original:

https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/core/CoreFeature.js#L11-L13

Changed:

// Added replace argument
reloadData(data, silent, columnsChanged, replace = false) {
  return this.table.dataLoader.load(data, undefined, undefined, replace, silent, columnsChanged);
}

Original:

https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Sort/Sort.js#L234-L244

Changed

refreshSort(){
  if(this.table.options.sortMode === "remote"){
      this.reloadData(null, false, false, true);  // Added last argument
  }else{
      this.refreshData(true);
  }

  //TODO - Persist left position of row manager
  // left = this.scrollLeft;
  // this.scrollHorizontal(left);
}

Original:

https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Filter/Filter.js#L571-L583

Changed:

refreshFilter(){
  if(this.tableInitialized){
      if(this.table.options.filterMode === "remote"){
          this.reloadData(null, false, false, true); // Added last argument
      }else{
          this.refreshData(true);
      }
  }

  //TODO - Persist left position of row manager
  // left = this.scrollLeft;
  // this.scrollHorizontal(left);
}

Original:

https://github.com/olifolkerd/tabulator/blob/7333c33730288f7f192943ebc3ed396b2e576fb8/src/js/modules/Page/Page.js#L773-L802

Changed:

case "remote":
  this.dataChanging = true;
  return this.reloadData(null, undefined, undefined, true)  // Added the last three arguments
      .finally(() => {
          this.dataChanging = false;
      });

I do however not know if these changes would break something else. @olifolkerd: If you think those changes would be ok, I would be happy to submit a PR 😊

Hi, this issue still in 6.2.1 (hope not fixed) i have done what you've exactly done, but instead of remote pagination i'm using progressive_load: scroll, for that what are values are need to be defined in reloadData()

Anton-Plagemann commented 4 months ago

Hi @Ariharan2002,

unfortunately I do not know what needs to be changed for it to work with progressive loading :/ I suggest you take a look by yourself or switch to pagination for now.

Ariharan2002 commented 4 months ago

Hi @Anton-Plagemann , BTW Thank you for your response :) , will take care of it

pollux2021 commented 3 months ago

I'm using v6.2.1 and the problem persists. anyone have any solution?

Ariharan2002 commented 3 months ago

Hi @pollux2021 , i have done like take the position of column which you are sorting and then after data sorted (use dataSorted event) use some amount of time in setTimeout block and then scroll to the clicked column, thats how i have worked and its working fine

leonardorame commented 2 months ago

Hi, just stumbled upon this same bug. This works well on 4.2.7, but not on 6.2.1.

leonardorame commented 2 months ago

Can confirm that 6.2.5 still has this issue.

leonardorame commented 2 months ago

Thanks @Anton-Plagemann for the fix, cloned Tabulator, configured build steps (https://tabulator.info/docs/6.2/build#setup), buit and replaced. Everything works as it should!.

Anton-Plagemann commented 4 days ago

@olifolkerd Any chance to add this fix? I would be happy to submit a pr 😊 I'm currently applying the fix to each version manually 😟

olifolkerd commented 4 days ago

@Anton-Plagemann a pr would be much appreciated, cheers

Oli