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

MultiGrid with fixedRowCount is rendering incorrect width #675

Merged
merged 1 commit into from
May 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions source/CellMeasurer/CellMeasurer.jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,10 @@ describe('CellMeasurer', () => {
(params) => <div style={{ width: 100, height: 30 }}></div>
)

let measurer;
const node = findDOMNode(render(
<CellMeasurer
ref={(ref) => { measurer = ref }}
cache={cache}
columnIndex={0}
parent={parent}
Expand All @@ -235,12 +237,43 @@ describe('CellMeasurer', () => {
</CellMeasurer>
))

node.style.width = 100
node.style.height = 30

child.mock.calls[0][0].measure()
const { height, width } = measurer._getCellMeasures(node)
expect(height).toBeGreaterThan(0)
expect(width).toBeGreaterThan(0)

expect(node.style.height).toBe('auto')
expect(node.style.width).not.toBe('auto')
})

// See issue #660
it('should set widht/height style to values before measuring with style "auto"', () => {
const cache = new CellMeasurerCache({
fixedHeight: true
})
const parent = createParent({ cache })
const child = jest.fn()
child.mockImplementation(
(params) => <div style={{ width: 100, height: 30 }}></div>
)

const node = findDOMNode(render(
<CellMeasurer
cache={cache}
columnIndex={0}
parent={parent}
rowIndex={0}
style={{}}
>
{child}
</CellMeasurer>
))

node.style.width = 200
node.style.height = 60

child.mock.calls[0][0].measure()

expect(node.style.height).toBe('30px')
expect(node.style.width).toBe('100px')
})
})
46 changes: 32 additions & 14 deletions source/CellMeasurer/CellMeasurer.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ export default class CellMeasurer extends PureComponent {
: children
}

_getCellMeasures (node) {
const { cache } = this.props

if (!cache.hasFixedWidth()) {
node.style.width = 'auto'
}
if (!cache.hasFixedHeight()) {
node.style.height = 'auto'
}

const height = Math.ceil(node.offsetHeight)
const width = Math.ceil(node.offsetWidth)

return { height, width };
}

_maybeMeasureCell () {
const {
cache,
Expand All @@ -51,23 +67,24 @@ export default class CellMeasurer extends PureComponent {

if (!cache.has(rowIndex, columnIndex)) {
const node = findDOMNode(this)
const oldWidth = node.style.width
const oldHeight = node.style.height

// TODO Check for a bad combination of fixedWidth and missing numeric width or vice versa with height

// Even if we are measuring initially- if we're inside of a MultiGrid component,
// Explicitly clear width/height before measuring to avoid being tainted by another Grid.
// eg top/left Grid renders before bottom/right Grid
// Since the CellMeasurerCache is shared between them this taints derived cell size values.
if (!cache.hasFixedWidth()) {
node.style.width = 'auto'
const { height, width } = this._getCellMeasures(node);

if (oldWidth) {
node.style.width = oldWidth
}
if (!cache.hasFixedHeight()) {
node.style.height = 'auto'
if (oldHeight) {
node.style.height = oldHeight
}

const height = Math.ceil(node.offsetHeight)
const width = Math.ceil(node.offsetWidth)

cache.set(
rowIndex,
columnIndex,
Expand Down Expand Up @@ -97,22 +114,23 @@ export default class CellMeasurer extends PureComponent {
} = this.props

const node = findDOMNode(this)
const oldWidth = node.style.width
const oldHeight = node.style.height

// If we are re-measuring a cell that has already been measured,
// It will have a hard-coded width/height from the previous measurement.
// The fact that we are measuring indicates this measurement is probably stale,
// So explicitly clear it out (eg set to "auto") so we can recalculate.
// See issue #593 for more info.
if (!cache.hasFixedWidth()) {
node.style.width = 'auto'
const { height, width } = this._getCellMeasures(node)

if (oldWidth) {
node.style.width = oldWidth
}
if (!cache.hasFixedHeight()) {
node.style.height = 'auto'
if (oldHeight) {
node.style.height = oldHeight
}

const height = Math.ceil(node.offsetHeight)
const width = Math.ceil(node.offsetWidth)

if (
height !== cache.getHeight(rowIndex, columnIndex) ||
width !== cache.getWidth(rowIndex, columnIndex)
Expand Down