From 97e319801a39321ad947443de515a8f93cd1c26b Mon Sep 17 00:00:00 2001 From: JF-Cozy Date: Tue, 14 Nov 2023 12:12:00 +0100 Subject: [PATCH] fix(BottomSheet): Content was not displayed correctly with toolbar --- react/BottomSheet/BottomSheet.jsx | 10 +-- react/BottomSheet/helpers.js | 23 ++++-- react/BottomSheet/helpers.spec.js | 131 ++++++++++++++++++++++++++++-- 3 files changed, 144 insertions(+), 20 deletions(-) diff --git a/react/BottomSheet/BottomSheet.jsx b/react/BottomSheet/BottomSheet.jsx index 335e0f6df3..a640e62f18 100644 --- a/react/BottomSheet/BottomSheet.jsx +++ b/react/BottomSheet/BottomSheet.jsx @@ -53,7 +53,7 @@ const ContainerWrapper = ({ showBackdrop, children }) => { } const createStyles = ({ - squared, + isTopPosition, hasToolbarProps, offset, renderSaferHeight, @@ -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 @@ -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 = @@ -171,7 +168,7 @@ const BottomSheet = memo( prevInitPos.current !== 0 && prevInitPos.current > initPos const styles = createStyles({ - squared, + isTopPosition, hasToolbarProps, offset, renderSaferHeight, @@ -235,6 +232,7 @@ const BottomSheet = memo( backdrop, maxHeight, innerContentHeight, + toolbarProps, offset }) const computedMediumHeight = computeMediumHeight({ diff --git a/react/BottomSheet/helpers.js b/react/BottomSheet/helpers.js index 1d2b20ff98..38c90f00fc 100644 --- a/react/BottomSheet/helpers.js +++ b/react/BottomSheet/helpers.js @@ -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 @@ -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 = ({ @@ -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) => diff --git a/react/BottomSheet/helpers.spec.js b/react/BottomSheet/helpers.spec.js index b923883376..18d6fa5395 100644 --- a/react/BottomSheet/helpers.spec.js +++ b/react/BottomSheet/helpers.spec.js @@ -5,6 +5,7 @@ import { setTopPosition, setBottomPosition, minimizeAndClose, + computeToolbarHeight, computeBottomSpacer } from './helpers' @@ -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, @@ -349,10 +362,22 @@ 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, @@ -360,10 +385,22 @@ describe('computeBottomSpacer', () => { 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, @@ -371,7 +408,19 @@ describe('computeBottomSpacer', () => { 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) }) }) }) @@ -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, @@ -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', () => { @@ -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', () => { @@ -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) + }) +})