Fix gallery image dimensions (#9247)
This commit is contained in:
parent
308aa0c45e
commit
c615e37982
4 changed files with 64 additions and 25 deletions
|
|
@ -2,8 +2,8 @@
|
||||||
// See LICENSE.txt for license information.
|
// See LICENSE.txt for license information.
|
||||||
|
|
||||||
import RNUtils from '@mattermost/rnutils';
|
import RNUtils from '@mattermost/rnutils';
|
||||||
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
|
import React, {useCallback, useEffect, useRef, useState} from 'react';
|
||||||
import {DeviceEventEmitter, Platform, StyleSheet, View} from 'react-native';
|
import {DeviceEventEmitter, Platform, View} from 'react-native';
|
||||||
|
|
||||||
import {Events} from '@constants';
|
import {Events} from '@constants';
|
||||||
import useAndroidHardwareBackHandler from '@hooks/android_back_handler';
|
import useAndroidHardwareBackHandler from '@hooks/android_back_handler';
|
||||||
|
|
@ -28,12 +28,6 @@ type Props = {
|
||||||
items: GalleryItemType[];
|
items: GalleryItemType[];
|
||||||
}
|
}
|
||||||
|
|
||||||
const styles = StyleSheet.create({
|
|
||||||
container: {
|
|
||||||
flex: 1,
|
|
||||||
},
|
|
||||||
});
|
|
||||||
|
|
||||||
const GalleryScreen = ({componentId, galleryIdentifier, hideActions, initialIndex, items}: Props) => {
|
const GalleryScreen = ({componentId, galleryIdentifier, hideActions, initialIndex, items}: Props) => {
|
||||||
const dim = useWindowDimensions();
|
const dim = useWindowDimensions();
|
||||||
const isTablet = useIsTablet();
|
const isTablet = useIsTablet();
|
||||||
|
|
@ -41,13 +35,7 @@ const GalleryScreen = ({componentId, galleryIdentifier, hideActions, initialInde
|
||||||
const {headerAndFooterHidden, hideHeaderAndFooter, headerStyles, footerStyles} = useGalleryControls();
|
const {headerAndFooterHidden, hideHeaderAndFooter, headerStyles, footerStyles} = useGalleryControls();
|
||||||
const galleryRef = useRef<GalleryRef>(null);
|
const galleryRef = useRef<GalleryRef>(null);
|
||||||
|
|
||||||
const containerStyle = useMemo(() => {
|
const containerStyle = dim;
|
||||||
if (Platform.OS === 'ios') {
|
|
||||||
return dim;
|
|
||||||
}
|
|
||||||
|
|
||||||
return styles.container;
|
|
||||||
}, [dim]);
|
|
||||||
|
|
||||||
const onClose = useCallback(() => {
|
const onClose = useCallback(() => {
|
||||||
// We keep the un freeze here as we want
|
// We keep the un freeze here as we want
|
||||||
|
|
@ -81,6 +69,10 @@ const GalleryScreen = ({componentId, galleryIdentifier, hideActions, initialInde
|
||||||
onClose();
|
onClose();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
if (Platform.OS === 'android' && Platform.Version >= 34) {
|
||||||
|
RNUtils.setNavigationBarColor('black', true);
|
||||||
|
}
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
listener.remove();
|
listener.remove();
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -1,19 +1,14 @@
|
||||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
||||||
// See LICENSE.txt for license information.
|
// See LICENSE.txt for license information.
|
||||||
|
|
||||||
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
|
|
||||||
// See LICENSE.txt for license information.
|
|
||||||
|
|
||||||
import {Image, type ImageSource} from 'expo-image';
|
import {Image, type ImageSource} from 'expo-image';
|
||||||
import React, {useEffect, useMemo, useState} from 'react';
|
import React, {useEffect, useMemo, useState} from 'react';
|
||||||
import {type ImageStyle, Platform, StyleSheet, View, type ViewStyle} from 'react-native';
|
import {type ImageStyle, StyleSheet, View, type ViewStyle} from 'react-native';
|
||||||
import Animated, {
|
import Animated, {
|
||||||
interpolate, runOnJS, runOnUI,
|
interpolate, runOnJS, runOnUI,
|
||||||
useAnimatedStyle, withTiming,
|
useAnimatedStyle, withTiming,
|
||||||
} from 'react-native-reanimated';
|
} from 'react-native-reanimated';
|
||||||
import {useSafeAreaInsets} from 'react-native-safe-area-context';
|
|
||||||
|
|
||||||
import {useDefaultHeaderHeight} from '@hooks/header';
|
|
||||||
import {calculateDimensions} from '@utils/images';
|
import {calculateDimensions} from '@utils/images';
|
||||||
|
|
||||||
import {pagerTimingConfig} from '../animation_config/timing';
|
import {pagerTimingConfig} from '../animation_config/timing';
|
||||||
|
|
@ -66,9 +61,6 @@ export default function Lightbox({
|
||||||
} = useLightboxSharedValues();
|
} = useLightboxSharedValues();
|
||||||
const [renderChildren, setRenderChildren] = useState<boolean>(false);
|
const [renderChildren, setRenderChildren] = useState<boolean>(false);
|
||||||
const childLayoutTimeoutRef = React.useRef<NodeJS.Timeout>();
|
const childLayoutTimeoutRef = React.useRef<NodeJS.Timeout>();
|
||||||
const insets = useSafeAreaInsets();
|
|
||||||
const headerHeight = useDefaultHeaderHeight() - insets.top;
|
|
||||||
const targetHeightDiff = Platform.OS === 'ios' ? 0 : (headerHeight / 2);
|
|
||||||
|
|
||||||
const animateOnMount = () => {
|
const animateOnMount = () => {
|
||||||
'worklet';
|
'worklet';
|
||||||
|
|
@ -94,6 +86,9 @@ export default function Lightbox({
|
||||||
childLayoutTimeoutRef.current = undefined;
|
childLayoutTimeoutRef.current = undefined;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// no need to add animateOnMount as this function uses references and is only needed on mount
|
||||||
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
const {width: tw, height: th} = useMemo(() => calculateDimensions(
|
const {width: tw, height: th} = useMemo(() => calculateDimensions(
|
||||||
|
|
@ -139,7 +134,7 @@ export default function Lightbox({
|
||||||
interpolate(animationProgress.value, [0, 1], range);
|
interpolate(animationProgress.value, [0, 1], range);
|
||||||
|
|
||||||
const targetX = (targetDimensions.width - tw) / 2;
|
const targetX = (targetDimensions.width - tw) / 2;
|
||||||
const targetY = ((targetDimensions.height - th) / 2) - targetHeightDiff;
|
const targetY = ((targetDimensions.height - th) / 2);
|
||||||
|
|
||||||
const top =
|
const top =
|
||||||
translateY.value +
|
translateY.value +
|
||||||
|
|
|
||||||
|
|
@ -197,6 +197,46 @@ describe('calculateDimensions', () => {
|
||||||
expect(result.width).toBe(300);
|
expect(result.width).toBe(300);
|
||||||
expect(result.height).toBe(300);
|
expect(result.height).toBe(300);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should constrain portrait image width when scaling to viewport height would cause overflow', () => {
|
||||||
|
// Portrait image 380×800, viewport 400×900
|
||||||
|
// Without fix: height=900, width=900×(380/800)=427.5 (OVERFLOW!)
|
||||||
|
// With fix: should scale based on width instead
|
||||||
|
const result = calculateDimensions(800, 380, 400, 900, true);
|
||||||
|
expect(result.width).toBeLessThanOrEqual(400);
|
||||||
|
expect(result.height).toBeLessThanOrEqual(900);
|
||||||
|
|
||||||
|
// Should constrain to width: 400w, height=400×(800/380)≈842
|
||||||
|
expect(result.width).toBe(400);
|
||||||
|
expect(result.height).toBeCloseTo(842.1, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should constrain landscape image height when scaling to viewport width would cause overflow', () => {
|
||||||
|
// Landscape image 800×380, viewport 900×400
|
||||||
|
// Without fix: width=900, height=900×(380/800)=427.5 (OVERFLOW!)
|
||||||
|
// With fix: should scale based on height instead
|
||||||
|
const result = calculateDimensions(380, 800, 900, 400, true);
|
||||||
|
expect(result.width).toBeLessThanOrEqual(900);
|
||||||
|
expect(result.height).toBeLessThanOrEqual(400);
|
||||||
|
|
||||||
|
// Should constrain to height: 400h, width=400×(800/380)≈842
|
||||||
|
expect(result.height).toBe(400);
|
||||||
|
expect(result.width).toBeCloseTo(842.1, 0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should handle extreme portrait aspect ratio with matchViewPort', () => {
|
||||||
|
// Very tall portrait image 200×1000 (1:5 ratio), viewport 300×600
|
||||||
|
// Would try to fit to height: 600h, width=600×(200/1000)=120 (OK)
|
||||||
|
// But if viewport is narrower: 100×600
|
||||||
|
// Would try: 600h, width=600×(200/1000)=120 (OVERFLOW!)
|
||||||
|
const result = calculateDimensions(1000, 200, 100, 600, true);
|
||||||
|
expect(result.width).toBeLessThanOrEqual(100);
|
||||||
|
expect(result.height).toBeLessThanOrEqual(600);
|
||||||
|
|
||||||
|
// Should constrain to width: 100w, height=100×(1000/200)=500
|
||||||
|
expect(result.width).toBe(100);
|
||||||
|
expect(result.height).toBe(500);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('getViewPortWidth', () => {
|
describe('getViewPortWidth', () => {
|
||||||
|
|
|
||||||
|
|
@ -57,10 +57,22 @@ export const calculateDimensions = (height?: number, width?: number, viewPortWid
|
||||||
// Portrait: fit to viewport height
|
// Portrait: fit to viewport height
|
||||||
imageHeight = viewPortHeight;
|
imageHeight = viewPortHeight;
|
||||||
imageWidth = imageHeight * heightRatio;
|
imageWidth = imageHeight * heightRatio;
|
||||||
|
|
||||||
|
// Ensure width doesn't exceed viewport
|
||||||
|
if (imageWidth > viewPortWidth) {
|
||||||
|
imageWidth = viewPortWidth;
|
||||||
|
imageHeight = imageWidth * ratio;
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
// Landscape or square: fit to viewport width
|
// Landscape or square: fit to viewport width
|
||||||
imageWidth = viewPortWidth;
|
imageWidth = viewPortWidth;
|
||||||
imageHeight = imageWidth * ratio;
|
imageHeight = imageWidth * ratio;
|
||||||
|
|
||||||
|
// Ensure height doesn't exceed viewport
|
||||||
|
if (imageHeight > viewPortHeight) {
|
||||||
|
imageHeight = viewPortHeight;
|
||||||
|
imageWidth = imageHeight * heightRatio;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue