Skip to content

Commit

Permalink
fix: MM-58164 Search text is behind the textbox while searching (#8026)
Browse files Browse the repository at this point in the history 
* fix: MM-58164 search text hides behind textbox
* clean up unit test for useCollapsibleHeader
* simplify math
* unlock doesn't need to use useCallback as it doesn't have any deps
* revert changes on unlock
* using lockValue & headerOffset directly into children props
* disable scrolling to prevent scrollValue from being updated
* properly snapping flatlist to the proper spot in a few scenarios

---------

Co-authored-by: Mattermost Build <[email protected]>
  • Loading branch information
@rahimrahman @mattermost-build
rahimrahman and mattermost-build authored Sep 5, 2024
1 parent 2e46f6c commit 00e05d5
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 36 deletions.
6 changes: 2 additions & 4 deletions app/components/navigation_header/header.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ type Props = {
onTitlePress?: () => void;
rightButtons?: HeaderRightButton[];
scrollValue?: Animated.SharedValue<number>;
lockValue?: Animated.SharedValue<number | null>;
showBackButton?: boolean;
subtitle?: string;
subtitleCompanion?: React.ReactElement;
Expand Down Expand Up @@ -135,7 +134,6 @@ const Header = ({
onTitlePress,
rightButtons,
scrollValue,
lockValue,
showBackButton = true,
subtitle,
subtitleCompanion,
Expand All @@ -155,7 +153,7 @@ const Header = ({
}

const barHeight = heightOffset - ViewConstants.LARGE_HEADER_TITLE_HEIGHT;
const val = (scrollValue?.value ?? 0);
const val = (scrollValue?.value || 0);
const showDuration = 200;
const hideDuration = 50;
const duration = val >= barHeight ? showDuration : hideDuration;
Expand All @@ -168,7 +166,7 @@ const Header = ({
const containerAnimatedStyle = useAnimatedStyle(() => ({
height: defaultHeight,
paddingTop: insets.top,
}), [defaultHeight, lockValue]);
}), [defaultHeight]);

const containerStyle = useMemo(() => (
[styles.container, containerAnimatedStyle]), [styles, containerAnimatedStyle]);
Expand Down
31 changes: 13 additions & 18 deletions app/components/navigation_header/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type Props = SearchProps & {
onTitlePress?: () => void;
rightButtons?: HeaderRightButton[];
scrollValue?: Animated.SharedValue<number>;
lockValue?: Animated.SharedValue<number | null>;
lockValue?: number;
hideHeader?: () => void;
showBackButton?: boolean;
subtitle?: string;
Expand Down Expand Up @@ -61,45 +61,40 @@ const NavigationHeader = forwardRef<SearchRef, Props>(({
const styles = getStyleSheet(theme);

const {largeHeight, defaultHeight, headerOffset} = useHeaderHeight();

const minScrollValue = useDerivedValue(() => scrollValue?.value || 0, [scrollValue]);

const containerHeight = useAnimatedStyle(() => {
const value = -(scrollValue?.value || 0);
const calculatedHeight = (isLargeTitle ? largeHeight : defaultHeight) + value;
const height = lockValue?.value ? lockValue.value : calculatedHeight;
const calculatedHeight = (isLargeTitle ? largeHeight : defaultHeight) - minScrollValue.value;
const height = lockValue || calculatedHeight;
return {
height: Math.max(height, defaultHeight),
minHeight: defaultHeight,
maxHeight: largeHeight + MAX_OVERSCROLL,
};
});

const minScrollValue = useDerivedValue(() => scrollValue?.value || 0, [scrollValue]);
}, [defaultHeight, largeHeight, lockValue, isLargeTitle]);

const translateY = useDerivedValue(() => (
lockValue?.value ? -lockValue.value : Math.min(-minScrollValue.value, headerOffset)
), [lockValue, minScrollValue, headerOffset]);
lockValue ? -lockValue : Math.min(-minScrollValue.value, headerOffset)
), [lockValue, headerOffset]);

const searchTopStyle = useAnimatedStyle(() => {
const margin = clamp(-minScrollValue.value, -headerOffset, headerOffset);
const marginTop = (lockValue?.value ? -lockValue?.value : margin) - SEARCH_INPUT_HEIGHT - SEARCH_INPUT_MARGIN;
const marginTop = (lockValue ? -lockValue : margin) - SEARCH_INPUT_HEIGHT - SEARCH_INPUT_MARGIN;
return {marginTop};
}, [lockValue, headerOffset, scrollValue]);

const heightOffset = useDerivedValue(() => (
lockValue?.value ? lockValue.value : headerOffset
), [lockValue, headerOffset]);
}, [lockValue, headerOffset]);

return (
<Animated.View style={[styles.container, containerHeight]}>
<Header
defaultHeight={defaultHeight}
hasSearch={hasSearch}
isLargeTitle={isLargeTitle}
heightOffset={heightOffset.value}
heightOffset={lockValue || headerOffset}
leftComponent={leftComponent}
onBackPress={onBackPress}
onTitlePress={onTitlePress}
rightButtons={rightButtons}
lockValue={lockValue}
scrollValue={scrollValue}
showBackButton={showBackButton}
subtitle={subtitle}
Expand All @@ -109,7 +104,7 @@ const NavigationHeader = forwardRef<SearchRef, Props>(({
/>
{isLargeTitle &&
<NavigationHeaderLargeTitle
heightOffset={heightOffset.value}
heightOffset={lockValue || headerOffset}
hasSearch={hasSearch}
subtitle={subtitle}
theme={theme}
Expand Down
15 changes: 9 additions & 6 deletions app/hooks/header.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.

import React, {useCallback, useMemo} from 'react';
import React, {useCallback, useMemo, useState} from 'react';
import Animated, {runOnJS, scrollTo, useAnimatedRef, useAnimatedScrollHandler, useDerivedValue, useSharedValue, withTiming} from 'react-native-reanimated';
import {useSafeAreaInsets} from 'react-native-safe-area-context';

Expand Down Expand Up @@ -52,13 +52,13 @@ export const useCollapsibleHeader = <T>(isLargeTitle: boolean, onSnap?: (offset:
const animatedRef = useAnimatedRef<Animated.ScrollView>();
const {largeHeight, defaultHeight, headerOffset} = useHeaderHeight();
const scrollValue = useSharedValue(0);
const lockValue = useSharedValue<number | null>(null);
const [lockValue, setLockValue] = useState<number>(0);
const autoScroll = useSharedValue(false);
const snapping = useSharedValue(false);
const scrollEnabled = useSharedValue(true);

const headerHeight = useDerivedValue(() => {
const value = -(scrollValue?.value || 0);
const value = -(scrollValue.value);
const heightWithScroll = (isLargeTitle ? largeHeight : defaultHeight) + value;
let height = Math.max(heightWithScroll, defaultHeight);
if (value > insets.top) {
Expand Down Expand Up @@ -134,11 +134,13 @@ export const useCollapsibleHeader = <T>(isLargeTitle: boolean, onSnap?: (offset:

const hideHeader = useCallback((lock = false) => {
if (lock) {
lockValue.value = defaultHeight;
// by disabling scroll, this prevent the scrollValue from being updated needlessly as observed in iOS simulators
scrollEnabled.value = false;
setLockValue(defaultHeight);
}

const offset = headerOffset;
if (animatedRef?.current && Math.abs((scrollValue?.value || 0)) <= insets.top) {
if (animatedRef?.current && Math.abs((scrollValue.value)) <= insets.top) {
autoScroll.value = true;
if ('scrollTo' in animatedRef.current) {
animatedRef.current.scrollTo({y: offset, animated: true});
Expand All @@ -154,7 +156,8 @@ export const useCollapsibleHeader = <T>(isLargeTitle: boolean, onSnap?: (offset:
}, [largeHeight, defaultHeight]);

const unlock = useCallback(() => {
lockValue.value = null;
scrollEnabled.value = true;
setLockValue(0);
}, []);

return {
Expand Down
88 changes: 88 additions & 0 deletions app/hooks/useCollapsibleHeader.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {renderHook} from '@testing-library/react-hooks';
import {act} from '@testing-library/react-native';

import ViewConstants from '@constants/view';

import * as DeviceFunctions from './device';
import {useCollapsibleHeader} from './header';

const LARGE_HEADER_TITLE_HEIGHT = 128;
const HEADER_OFFSET = LARGE_HEADER_TITLE_HEIGHT - ViewConstants.DEFAULT_HEADER_HEIGHT;

describe('useCollapsibleHeader', () => {
afterEach(() => {
jest.resetAllMocks();
});

const commonHookResponse = {
largeHeight: LARGE_HEADER_TITLE_HEIGHT,
scrollRef: {current: null},
scrollValue: {value: 0},
onScroll: expect.any(Function),
hideHeader: expect.any(Function),
lockValue: 0,
unlock: expect.any(Function),
headerOffset: HEADER_OFFSET,
scrollEnabled: {value: true},
setAutoScroll: expect.any(Function),
};

it('should return the correct values with isLargeTitle is true', () => {
const {result} = renderHook(() => useCollapsibleHeader(true));

expect(result.current).toEqual({
defaultHeight: ViewConstants.DEFAULT_HEADER_HEIGHT,
scrollPaddingTop: LARGE_HEADER_TITLE_HEIGHT,
headerHeight: {value: LARGE_HEADER_TITLE_HEIGHT},
...commonHookResponse,
});
});

it('should return the correct values with isLargeTitle is false', () => {
const {result} = renderHook(() => useCollapsibleHeader(false));

expect(result.current).toEqual({
defaultHeight: ViewConstants.DEFAULT_HEADER_HEIGHT,
scrollPaddingTop: ViewConstants.DEFAULT_HEADER_HEIGHT,
headerHeight: {value: ViewConstants.DEFAULT_HEADER_HEIGHT},
...commonHookResponse,
});
});

it('should return the correct values with isLargeTitle is true, and on a tablet', () => {
jest.spyOn(DeviceFunctions, 'useIsTablet').mockReturnValue(true);

const {result} = renderHook(() => useCollapsibleHeader(true));

expect(result.current).toEqual({
defaultHeight: ViewConstants.TABLET_HEADER_HEIGHT,
scrollPaddingTop: LARGE_HEADER_TITLE_HEIGHT,
headerHeight: {value: LARGE_HEADER_TITLE_HEIGHT},
...commonHookResponse,
});
});

it('should change the lock value when hideHeader is called', () => {
const {result} = renderHook(() => useCollapsibleHeader(true));

expect(result.current.lockValue).toBe(0);

act(() => result.current.hideHeader(true));

expect(result.current.lockValue).toBe(ViewConstants.DEFAULT_HEADER_HEIGHT);
});

it('should reset the lockValue when unlock is called', () => {
const {result} = renderHook(() => useCollapsibleHeader(true));

act(() => result.current.hideHeader(true));

expect(result.current.lockValue).toBe(ViewConstants.DEFAULT_HEADER_HEIGHT);

act(() => result.current.unlock());

expect(result.current.lockValue).toBe(0);
});
});
22 changes: 14 additions & 8 deletions app/screens/home/search/search.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,13 @@ const SearchScreen = ({teamId, teams}: Props) => {
scrollRef.current?.scrollToOffset({offset, animated});
};

const onSnapWithTimeout = useCallback((offset: number, animated = true) => {
// wait until the keyboard is completely dismissed before scrolling to where the header should be
setTimeout(() => onSnap(offset, animated), 100);
}, [onSnap]);

const {
headerHeight,
headerOffset,
hideHeader,
lockValue,
onScroll,
Expand Down Expand Up @@ -156,7 +160,6 @@ const SearchScreen = ({teamId, teams}: Props) => {
const handleCancelSearch = useCallback(() => {
cancelRef.current = true;
resetToInitial();
onSnap(0);
}, [resetToInitial]);

const handleTextChange = useCallback((newValue: string) => {
Expand Down Expand Up @@ -204,7 +207,10 @@ const SearchScreen = ({teamId, teams}: Props) => {

const onBlur = useCallback(() => {
setSearchIsFocused(false);
}, []);
if (!cancelRef.current && !clearRef.current) {
onSnapWithTimeout(0);
}
}, [onSnapWithTimeout]);

const onFocus = useCallback(() => {
setSearchIsFocused(true);
Expand Down Expand Up @@ -281,7 +287,7 @@ const SearchScreen = ({teamId, teams}: Props) => {
}, [isFocused, stateIndex]);

const headerTopStyle = useAnimatedStyle(() => ({
top: lockValue.value ? lockValue.value : headerHeight.value,
top: lockValue || headerHeight.value,
zIndex: lastSearchedValue ? 10 : 0,
}), [headerHeight, lastSearchedValue, lockValue]);

Expand All @@ -304,15 +310,15 @@ const SearchScreen = ({teamId, teams}: Props) => {
const onFlatLayout = useCallback(() => {
if (clearRef.current || cancelRef.current) {
unlock();
onSnapWithTimeout(0);
}

if (clearRef.current) {
onSnap(headerOffset, false);
clearRef.current = false;
} else if (cancelRef.current) {
onSnap(0);
cancelRef.current = false;
}
}, [headerOffset, scrollRef]);
}, [unlock, onSnapWithTimeout]);

useDidUpdate(() => {
if (isFocused) {
Expand Down Expand Up @@ -407,7 +413,7 @@ const SearchScreen = ({teamId, teams}: Props) => {
posts={posts}
matches={matches}
fileInfos={fileInfos}
scrollPaddingTop={lockValue.value}
scrollPaddingTop={lockValue}
fileChannelIds={fileChannelIds}
/>
}
Expand Down

0 comments on commit 00e05d5

Please sign in to comment.