Skip to content

Commit

Permalink
fix(HeightAnimation): enhance calculation of height (#3335)
Browse files Browse the repository at this point in the history 
By using `position: absolute` on the first pain styles, we got a wrong
height calculation.
  • Loading branch information
@tujoworker
tujoworker authored Feb 28, 2024
1 parent 196bc7f commit e1a9859
Show file tree
Hide file tree
Showing 4 changed files with 288 additions and 59 deletions.
28 changes: 16 additions & 12 deletions packages/dnb-eufemia/src/components/height-animation/HeightAnimationInstance.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ export default class HeightAnimation {
isAnimating: boolean
__currentHeight: number

firstPaintStyle = {
visibility: 'hidden',
opacity: '0', // prevents before/after elements to be visible
height: 'auto',
}

constructor(opts: HeightAnimationOptions = {}) {
this.isInBrowser = typeof window !== 'undefined'
this.setState('init')
Expand Down Expand Up @@ -164,14 +170,6 @@ export default class HeightAnimation {
getHeight() {
return parseFloat(String(this.elem?.clientHeight)) || null
}
firstPaintStyle() {
return {
position: 'absolute',
visibility: 'hidden',
opacity: '0', // prevents before/after elements to be visible
height: 'auto',
}
}
getUnknownHeight() {
if (!this.elem) {
return null
Expand All @@ -181,13 +179,15 @@ export default class HeightAnimation {
return this.__currentHeight
}

const width = this.elem.clientWidth
const clonedElem = this.elem.cloneNode(true) as HTMLElement
this.elem.parentNode?.insertBefore(clonedElem, this.elem.nextSibling)

for (const key in this.firstPaintStyle) {
clonedElem.style[key] = this.firstPaintStyle[key]
}
clonedElem.style.height = 'auto'
clonedElem.style.width = width ? `${String(width)}px` : 'auto' // set width because of the "position: absolute"
clonedElem.style.position = 'absolute' // not a part of the "firstPaintStyle"

const height =
parseFloat(String(clonedElem.clientHeight)) ||
Expand Down Expand Up @@ -228,6 +228,7 @@ export default class HeightAnimation {
return
}

this.stop()
this.isAnimating = true

// make the animation
Expand Down Expand Up @@ -275,7 +276,7 @@ export default class HeightAnimation {
const toHeight = this.getUnknownHeight()

this.addEndEvent((e) => {
if (e.target === e.currentTarget) {
if (e.target === e.currentTarget || !e.currentTarget) {
this.setState('opened')
this.readjust()
}
Expand All @@ -299,7 +300,7 @@ export default class HeightAnimation {
const fromHeight = this.getHeight()

this.addEndEvent((e) => {
if (e.target === e.currentTarget) {
if (e.target === e.currentTarget || !e.currentTarget) {
if (this.elem) {
this.elem.style.visibility = 'hidden'
this.elem.style.overflowY = 'clip'
Expand Down Expand Up @@ -342,7 +343,10 @@ export default class HeightAnimation {
this.callAnimationStart()

this.addEndEvent((e) => {
if (this.state === 'adjusting' && e.target === e.currentTarget) {
if (
this.state === 'adjusting' &&
(e.target === e.currentTarget || !e.currentTarget)
) {
if (this.elem) {
this.elem.style.height = 'auto'
}
Expand Down
230 changes: 197 additions & 33 deletions ...ges/dnb-eufemia/src/components/height-animation/__tests__/HeightAnimationInstance.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { wait } from '../../../core/jest/jestSetup'
import HeightAnimationInstance from '../HeightAnimationInstance'
import {
simulateAnimationEnd,
Expand Down Expand Up @@ -37,6 +38,20 @@ describe('HeightAnimationInstance', () => {
expect(inst.elem).toBeUndefined()
})

it('firstPaintStyle should have these properties', () => {
const inst = new HeightAnimationInstance()
expect(inst.firstPaintStyle).toEqual({
height: 'auto',
opacity: '0',
visibility: 'hidden',
})
expect(inst.firstPaintStyle).not.toEqual(
expect.objectContaining({
position: 'absolute',
})
)
})

it('getHeight should return height', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)
Expand All @@ -46,55 +61,115 @@ describe('HeightAnimationInstance', () => {
expect(inst.getHeight()).toBe(100)
})

it('getUnknownHeight should return proper height', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)
describe('getUnknownHeight', () => {
it('should return proper height', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

mockHeight(100, element)
mockHeight(100, element)

expect(inst.getUnknownHeight()).toBe(100)
})
expect(inst.getUnknownHeight()).toBe(100)
})

it('open should call getUnknownHeight', () => {
const inst = new HeightAnimationInstance()
it('should create a cloned element', async () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

jest.spyOn(inst, 'getUnknownHeight').mockImplementation(jest.fn())
mockHeight(100, element)

inst.setElement(element)
inst.setState('closed')
inst.open()
const addedNodes = []
const removedNodes = []

const observer = new MutationObserver((mutationsList) => {
for (const mutation of mutationsList) {
if (mutation.type === 'childList') {
if (mutation.removedNodes?.length) {
removedNodes.push(mutation.removedNodes)
}
if (mutation.addedNodes?.length) {
addedNodes.push(mutation.addedNodes)
}
}
}
})

expect(inst.getUnknownHeight).toHaveBeenCalledTimes(1)
})
observer.observe(document.body, {
childList: true,
})

it('getUnknownHeight should use cached height during animation', () => {
const inst = new HeightAnimationInstance()
inst.getUnknownHeight()

mockHeight(100, element)
await wait(1)

expect(inst.__currentHeight).toBe(undefined)
expect(inst.isAnimating).toBe(undefined)
observer.disconnect()

inst.setElement(element)
inst.setState('closed')
inst.open()
expect(addedNodes).toHaveLength(1)
expect(removedNodes).toHaveLength(1)
})

expect(inst.isAnimating).toBe(true)
expect(inst.__currentHeight).toBe(100)
it('should create a cloned element with firstPaintStyle styles', async () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

mockHeight(200, element)
mockHeight(100, element)

inst.getUnknownHeight()
const styles = []

expect(inst.__currentHeight).toBe(100)
const observer = new MutationObserver((mutationsList) => {
for (const mutation of mutationsList) {
if (mutation.type === 'childList') {
if (mutation.addedNodes?.length) {
styles.push(mutation.addedNodes[0])
}
}
}
})

observer.observe(document.body, {
childList: true,
})

inst.getUnknownHeight()

await wait(1)

observer.disconnect()

expect(styles).toHaveLength(1)
expect(styles[0].getAttribute('style')).toBe(
'visibility: hidden; opacity: 0; height: auto; width: auto; position: absolute;'
)
})

delete inst.elem
it('should use cached height during animation', () => {
const inst = new HeightAnimationInstance()

expect(inst.getUnknownHeight()).toBe(null)
mockHeight(100, element)

inst.callAnimationEnd()
expect(inst.__currentHeight).toBe(undefined)
expect(inst.isAnimating).toBe(undefined)

expect(inst.__currentHeight).toBe(undefined)
inst.setElement(element)
inst.setState('closed')
inst.open()

expect(inst.isAnimating).toBe(true)
expect(inst.__currentHeight).toBe(100)

mockHeight(200, element)

inst.getUnknownHeight()

expect(inst.__currentHeight).toBe(100)

delete inst.elem

expect(inst.getUnknownHeight()).toBe(null)

inst.callAnimationEnd()

expect(inst.__currentHeight).toBe(undefined)
})
})

describe('start', () => {
Expand Down Expand Up @@ -146,14 +221,103 @@ describe('HeightAnimationInstance', () => {
expect(onStart).toHaveBeenCalledTimes(0)
expect(onEnd).toHaveBeenCalledTimes(0)
})

it('should call stop', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

jest.spyOn(inst, 'stop').mockImplementation(jest.fn())
inst.start(100, 200)

expect(inst.stop).toHaveBeenCalledTimes(1)
})

it('should set reqId1 and reqId2', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

inst.start(100, 200)

expect(inst.reqId1).toBe(1)
expect(inst.reqId2).toBeUndefined()

nextAnimationFrame()

expect(inst.reqId1).toBe(1)
expect(inst.reqId2).toBe(1)
})

it('should set height style', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

inst.start(100, 200)

expect(inst.elem.style.height).toBe('')

nextAnimationFrame()

expect(inst.elem.style.height).toBe('100px')

nextAnimationFrame()

expect(inst.elem.style.height).toBe('200px')
})

it('should set not height style when element is missing', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)
const elem = inst.elem

inst.start(100, 200)

expect(elem.style.height).toBe('')

nextAnimationFrame()

expect(elem.style.height).toBe('100px')

inst.elem = undefined // here we remove the element during the second animation frame
nextAnimationFrame()

expect(elem.style.height).toBe('100px')
})

it('should not run when element is not set', () => {
const inst = new HeightAnimationInstance()
inst.start(100, 200)
expect(inst.reqId1).toBeUndefined()
expect(inst.reqId2).toBeUndefined()
})

it('should set isAnimating to true', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

inst.start(100, 200)

expect(inst.isAnimating).toBe(true)
})
})

describe('open', () => {
beforeEach(() => {
globalThis.bypassTime = 1
})

it('should call setAsOpen when criterias are met', () => {
it('should call getUnknownHeight', () => {
const inst = new HeightAnimationInstance()

jest.spyOn(inst, 'getUnknownHeight').mockImplementation(jest.fn())

inst.setElement(element)
inst.setState('closed')
inst.open()

expect(inst.getUnknownHeight).toHaveBeenCalledTimes(1)
})

it('should call setAsOpen when criteria are met', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

Expand Down Expand Up @@ -256,7 +420,7 @@ describe('HeightAnimationInstance', () => {
globalThis.bypassTime = 1
})

it('should call setAsClosed when criterias are met', () => {
it('should call setAsClosed when criteria are met', () => {
const inst = new HeightAnimationInstance()
inst.setElement(element)

Expand Down
Loading

0 comments on commit e1a9859

Please sign in to comment.