Fix header and footer height calculation issue with css transform scale#4846
Conversation
…Rect when using fitColumns-layout-mode
There was a problem hiding this comment.
@plc-dev I think this looks much better! Two things I'd suggest adjusting:
- A single source of scale multipliers
This could be simpler by adding scaleX and scaleY as properties in Tabulator class and use it everywhere. For example:
export default class Column extends CoreFeature {
// ...
getHeight(){
return this.element.getBoundingClientRect().height * this.table.scaleY;
}
}scaleY would be calculated as getBoundingClientRect().height / offsetHeight on the table element (or maybe the RowManager?). scaleX would be calculated similarly. That should match the way CodeMirror 6 works. Also, please make sure that these are updated properly.
- Please remove the changes in the
distfolder. These will be changed when there is a new release.
| //resize columns to fit | ||
| export default function(columns, forced){ | ||
| var totalWidth = this.table.rowManager.element.getBoundingClientRect().width; //table element width | ||
| var totalWidth = Helpers.getCorrectedDimensions(this.table.rowManager.element, 'width'); //table element width |
There was a problem hiding this comment.
Instead of calling Helpers here, could you add a new getWidth method to the RowManager? So this will be:
var totalWidth = this.table.rowManager.getWidth();| let otherHeight = Math.floor(this.table.columnManager.getElement().getBoundingClientRect().height + (this.table.footerManager && this.table.footerManager.active && !this.table.footerManager.external ? this.table.footerManager.getElement().getBoundingClientRect().height : 0)); | ||
| let columnHeight = Helpers.getCorrectedDimensions(this.table.columnManager.getElement(), 'height'); | ||
| let footerHeight = (this.table.footerManager && this.table.footerManager.active && !this.table.footerManager.external) ? | ||
| Helpers.getCorrectedDimensions(this.table.footerManager.getElement(), 'height') : 0; |
There was a problem hiding this comment.
Same as RowManager, please add a new getHeight for FooterManager.
| @@ -1043,7 +1043,10 @@ export default class RowManager extends CoreFeature{ | |||
| let resized = false; | |||
|
|
|||
| if(this.renderer.verticalFillMode === "fill"){ | |||
| let otherHeight = Math.floor(this.table.columnManager.getElement().getBoundingClientRect().height + (this.table.footerManager && this.table.footerManager.active && !this.table.footerManager.external ? this.table.footerManager.getElement().getBoundingClientRect().height : 0)); | |||
| let columnHeight = Helpers.getCorrectedDimensions(this.table.columnManager.getElement(), 'height'); | |||
There was a problem hiding this comment.
Please add getHeight to the ColumnManager.
82a85b7 to
2a742cb
Compare
Done
I'm afraid calculating both of the scale-factors based on the dimensions of the element-property of the tabulator-class does not lead to the desired result, as atleast the scaleY-factor depends on the dimensions of the element-property of the columnHeader or the footerHeader.
As I'm unsure of the implications of turning autoresize off and manually calling the redraw function in userland, maybe it's preferable to put the scale-values into methods that recompute them on call (e.g. getScaleX). Let me know your preferences and I'll add the changes accordingly. |
| columnRect = column.getElement().getBoundingClientRect(); | ||
| rowManagerRect = this.table.rowManager.getElement().getBoundingClientRect(); | ||
| columnManagerRect = this.table.columnManager.getElement().getBoundingClientRect(); | ||
| rowRect = Helpers.getCorrectedRect(row.getElement()); |
There was a problem hiding this comment.
This is actually redundant, as the default case of getCorrectedRect returns the boundingClientRect of the passed HTMLElement, so I'll revert those changes.
…hen selecting ranges" This reverts commit 5fe5704.
Reimplemented the solution proposed by @azmy60. See below discussion of issue #4804 (comment)
Since this issue seems to be relevant to more people, I opened another PR that adresses the prior critique (see #4845) and uses the suggested approach. Let me know, if there are other fundamental issues and I'll try to solve them in a timely manner.