Skip to content

Commit

Permalink
fix(BottomSheet): Content was not displayed correctly with toolbar
Browse files Browse the repository at this point in the history
  • Loading branch information
JF-Cozy committed Nov 14, 2023
1 parent 11af87c commit 97e3198
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 20 deletions.
10 changes: 4 additions & 6 deletions react/BottomSheet/BottomSheet.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const ContainerWrapper = ({ showBackdrop, children }) => {
}

const createStyles = ({
squared,
isTopPosition,
hasToolbarProps,
offset,
renderSaferHeight,
Expand All @@ -67,7 +67,7 @@ const createStyles = ({
'0 -0.5px 0px 0 rgba(0, 0, 0, 0.10), 0 -2px 2px 0 rgba(0, 0, 0, 0.02), 0 -4px 4px 0 rgba(0, 0, 0, 0.02), 0 -8px 8px 0 rgba(0, 0, 0, 0.02), 0 -16px 16px 0 rgba(0, 0, 0, 0.02)',
backgroundColor: 'var(--paperBackgroundColor)',
zIndex: 'var(--zIndex-modal)',
...(squared && {
...(isTopPosition && {
borderTopLeftRadius: 0,
borderTopRightRadius: 0,
boxShadow: hasToolbarProps
Expand Down Expand Up @@ -160,9 +160,6 @@ const BottomSheet = memo(
const [initPos, setInitPos] = useState(0)
const prevInitPos = useRef()

const squared = backdrop
? isTopPosition && bottomSpacerHeight <= 0
: isTopPosition
const hasToolbarProps = !!Object.keys(toolbarProps).length
const isClosable = !!onClose || backdrop
const renderSaferHeight =
Expand All @@ -171,7 +168,7 @@ const BottomSheet = memo(
prevInitPos.current !== 0 && prevInitPos.current > initPos

const styles = createStyles({
squared,
isTopPosition,
hasToolbarProps,
offset,
renderSaferHeight,
Expand Down Expand Up @@ -235,6 +232,7 @@ const BottomSheet = memo(
backdrop,
maxHeight,
innerContentHeight,
toolbarProps,
offset
})
const computedMediumHeight = computeMediumHeight({
Expand Down
23 changes: 16 additions & 7 deletions react/BottomSheet/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { getFlagshipMetadata } from 'cozy-device-helper'
import { ANIMATION_DURATION } from './constants'
import { getSafeAreaValue } from '../helpers/getSafeArea'

export const computeMaxHeight = toolbarProps => {
export const computeToolbarHeight = (toolbarProps = {}) => {
const { ref, height } = toolbarProps
let computedToolbarHeight = 1

Expand All @@ -14,7 +14,13 @@ export const computeMaxHeight = toolbarProps => {
computedToolbarHeight = ref.current.offsetHeight
}

return window.innerHeight - computedToolbarHeight
return computedToolbarHeight
}

export const computeMaxHeight = toolbarProps => {
const toolbarHeight = computeToolbarHeight(toolbarProps)

return window.innerHeight - toolbarHeight
}

export const computeMediumHeight = ({
Expand Down Expand Up @@ -125,16 +131,19 @@ export const computeBottomSpacer = ({
backdrop,
maxHeight,
innerContentHeight,
toolbarProps,
offset
}) => {
// "maxHeight - innerContentHeight <= 0" happens for
// content longer than the window
if (maxHeight - innerContentHeight <= 0 || backdrop) {
return offset
// "maxHeight - innerContentHeight <= 0" happens for content longer than the available height
// such as height window or height window minus toolbar height
if (maxHeight - innerContentHeight <= 0) {
const toolbarHeight = computeToolbarHeight(toolbarProps)

return offset + toolbarHeight
}

// without backdrop, we want the bottomsheet to open to the top of the window
return maxHeight - innerContentHeight
return backdrop ? offset : maxHeight - innerContentHeight
}

export const getCssValue = (element, value) =>
Expand Down
131 changes: 124 additions & 7 deletions react/BottomSheet/helpers.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
setTopPosition,
setBottomPosition,
minimizeAndClose,
computeToolbarHeight,
computeBottomSpacer
} from './helpers'

Expand Down Expand Up @@ -339,6 +340,18 @@ describe('computeBottomSpacer', () => {
expect(res).toBe(600)
})

it('should return maxHeight - innerContentHeight if no offset even with toolbar', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 200,
toolbarProps: { height: 50 },
offset: 0
})

expect(res).toBe(600)
})

it('should return maxHeight - innerContentHeight even with an offset', () => {
const res = computeBottomSpacer({
backdrop: false,
Expand All @@ -349,29 +362,65 @@ describe('computeBottomSpacer', () => {

expect(res).toBe(600)
})

it('should return maxHeight - innerContentHeight even with an offset and toolbar', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 200,
toolbarProps: { height: 50 },
offset: 48
})

expect(res).toBe(600)
})
})

describe('inner content shorter than max height', () => {
it('should', () => {
it('should return the offset value and border even if zero', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 1000,
offset: 0
})

expect(res).toBe(0)
expect(res).toBe(1)
})

it('should return the toolbar height', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 1000,
toolbarProps: { height: 50 },
offset: 0
})

expect(res).toBe(50)
})

it('should', () => {
it('should return the offset value and border', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 1000,
offset: 48
})

expect(res).toBe(48)
expect(res).toBe(49)
})

it('should return the offset value and border and toolbar height', () => {
const res = computeBottomSpacer({
backdrop: false,
maxHeight: 800,
innerContentHeight: 1000,
toolbarProps: { height: 50 },
offset: 48
})

expect(res).toBe(98)
})
})
})
Expand All @@ -389,7 +438,19 @@ describe('computeBottomSpacer', () => {
expect(res).toBe(0)
})

it('should return the offset value ', () => {
it('should return the offset value even if zero and toolbar', () => {
const res = computeBottomSpacer({
backdrop: true,
maxHeight: 800,
innerContentHeight: 200,
toolbarProps: { height: 50 },
offset: 0
})

expect(res).toBe(0)
})

it('should return the offset value', () => {
const res = computeBottomSpacer({
backdrop: true,
maxHeight: 800,
Expand All @@ -399,6 +460,18 @@ describe('computeBottomSpacer', () => {

expect(res).toBe(48)
})

it('should return the offset value even with toolbar', () => {
const res = computeBottomSpacer({
backdrop: true,
maxHeight: 800,
innerContentHeight: 200,
toolbarProps: { height: 50 },
offset: 48
})

expect(res).toBe(48)
})
})

describe('inner content shorter than max height', () => {
Expand All @@ -410,7 +483,19 @@ describe('computeBottomSpacer', () => {
offset: 0
})

expect(res).toBe(0)
expect(res).toBe(1)
})

it('should return the toolbar height', () => {
const res = computeBottomSpacer({
backdrop: true,
maxHeight: 800,
innerContentHeight: 1000,
toolbarProps: { height: 50 },
offset: 0
})

expect(res).toBe(50)
})

it('should return the offset value', () => {
Expand All @@ -421,8 +506,40 @@ describe('computeBottomSpacer', () => {
offset: 48
})

expect(res).toBe(48)
expect(res).toBe(49)
})

it('should return the offset value and toolbar height', () => {
const res = computeBottomSpacer({
backdrop: true,
maxHeight: 800,
innerContentHeight: 1000,
toolbarProps: { height: 50 },
offset: 48
})

expect(res).toBe(98)
})
})
})
})

describe('computeToolbarHeight', () => {
it('should return the height prop', () => {
const res = computeToolbarHeight({ height: 50 })

expect(res).toBe(50)
})

it('should return the height from ref', () => {
const res = computeToolbarHeight({ ref: { current: { offsetHeight: 50 } } })

expect(res).toBe(50)
})

it('should return default value', () => {
const res = computeToolbarHeight()

expect(res).toBe(1)
})
})

0 comments on commit 97e3198

Please sign in to comment.