ResizeObserver: Use contentBoxSize #2882
Conversation
src/hooks/useGridDimensions.ts
Outdated
|
|
||
| saveDimensions(); | ||
| const resizeObserver = new ResizeObserver(saveDimensions); | ||
| setInlineSize(size.inlineSize - (devicePixelRatio % 1 === 0 ? 0 : 1)); |
There was a problem hiding this comment.
As we are using inlineSize/blockSize, RDG is one step closer to handling writing mode
https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserverEntry/contentBoxSize#value
There was a problem hiding this comment.
Can we check that these values are the same as clientWidth/clientHeight?
Something like
if (clientWidth !== inlineSize || clientHeight !== blockSize) {
throw new Error('🤔');
}Just to make sure, not to be commited.
There was a problem hiding this comment.
I guess we can give it a try, but I'm worried about decimal values leading to more shaky grids.
There was a problem hiding this comment.
If we have a reproducible bug example then we can test it
Codecov Report
@@ Coverage Diff @@
## main #2882 +/- ##
==========================================
+ Coverage 96.22% 96.24% +0.01%
==========================================
Files 38 38
Lines 1244 1250 +6
Branches 391 391
==========================================
+ Hits 1197 1203 +6
Misses 47 47
|
| this.callback([], this); | ||
| // patch inlineSize/blockSize to pretend we're rendering DataGrid at 1920p/1080p | ||
| // @ts-expect-error | ||
| this.callback([{ contentBoxSize: [{ inlineSize: 1920, blockSize: 1080 }] }], this); |
There was a problem hiding this comment.
Can we remove the clientWidth/clientHeight patch below?
There was a problem hiding this comment.
It is still needed. We can also change the window.ResizeObserver to not call callback
Co-authored-by: Nicolas Stepien <[email protected]>
Co-authored-by: Nicolas Stepien <[email protected]>
FF bug is fixed in 99. We can wait for a few weeks before merging this PR to give everyone some time