Skip to content

Enable keyboard avoiding view in search modal #57458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Gonals merged 5 commits into from
Mar 3, 2025
Merged

Enable keyboard avoiding view in search modal #57458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
276731b
enable keyboard avoiding view in search modal
bernhardoj Feb 26, 2025
fa57cf3
Merge branch 'main' into fix/56342-can't-scroll-search-results-when-k…
bernhardoj Feb 26, 2025
8b04d0b
Merge branch 'main' into fix/56342-can't-scroll-search-results-when-k…
bernhardoj Feb 28, 2025
52096f5
keyboard avoiding view in mweb safari
bernhardoj Feb 28, 2025
abf96de
prevent viewport scrolling
bernhardoj Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Search/SearchAutocompleteList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ function SearchAutocompleteList(
onSelectRow={onListItemPress}
ListItem={SearchRouterItem}
containerStyle={[styles.mh100]}
sectionListStyle={[styles.ph2, styles.pb2]}
sectionListStyle={[styles.ph2, styles.pb2, styles.overscrollBehaviorContain]}

@eh2077 eh2077 Mar 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we apply overscrollBehaviorContain only for mobile WebKit? I found a use case here.

App/src/components/ScreenWrapper.tsx

Line 284 in cd92ca9

const isAvoidingViewportScroll = useTackInputFocus(isFocused && shouldEnableMaxHeight && shouldAvoidScrollOnVirtualViewport && isMobileWebKit());

@eh2077 eh2077 Mar 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retested other platforms with different use cases, with few contacts (7-9) or long contact list. I didn't find any inconsistencies, so these comments won't be a blocker. They're just for discussing

@bernhardoj bernhardoj Mar 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any issues on other platforms too, so I think we can just leave it, same as maxHeight above.

They probably add it for iOS mWeb only since it's the only platform where the viewport scroll issue happens, but since it doesn't cause any inconsistency on other platforms, I think it's fine to apply it to all. It's actually more consistent if we apply the overscroll style to all platforms instead of only 1 platform.

listItemWrapperStyle={[styles.pr0, styles.pl0]}
getItemHeight={getItemHeight}
onLayout={() => {
Expand Down
26 changes: 18 additions & 8 deletions src/components/Search/SearchRouter/SearchRouterModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import React, {useState} from 'react';
import FocusTrapForModal from '@components/FocusTrap/FocusTrapForModal';
import KeyboardAvoidingView from '@components/KeyboardAvoidingView';
import Modal from '@components/Modal';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useSafeAreaInsets from '@hooks/useSafeAreaInsets';
import useThemeStyles from '@hooks/useThemeStyles';
import useViewportOffsetTop from '@hooks/useViewportOffsetTop';
import useWindowDimensions from '@hooks/useWindowDimensions';
import {isMobileChrome, isMobileSafari} from '@libs/Browser';
import CONST from '@src/CONST';
import SearchRouter from './SearchRouter';
Expand All @@ -12,7 +15,9 @@ import {useSearchRouterContext} from './SearchRouterContext';
const isMobileWebSafari = isMobileSafari();

function SearchRouterModal() {
const styles = useThemeStyles();
const {shouldUseNarrowLayout} = useResponsiveLayout();
const {windowHeight} = useWindowDimensions();
const {isSearchRouterDisplayed, closeSearchRouter} = useSearchRouterContext();
const viewportOffsetTop = useViewportOffsetTop();
const safeAreaInsets = useSafeAreaInsets();
Expand All @@ -35,14 +40,19 @@ function SearchRouterModal() {
onModalHide={() => setShouldHideInputCaret(isMobileWebSafari)}
onModalShow={() => setShouldHideInputCaret(false)}
>
{isSearchRouterDisplayed && (
<FocusTrapForModal active={isSearchRouterDisplayed}>
<SearchRouter
onRouterClose={closeSearchRouter}
shouldHideInputCaret={shouldHideInputCaret}
/>
</FocusTrapForModal>
)}
<KeyboardAvoidingView
behavior="padding"
style={[styles.flex1, {maxHeight: windowHeight}]}

@eh2077 eh2077 Mar 3, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only add maxHeight: windowHeight for mobile Safari?

>
{isSearchRouterDisplayed && (
<FocusTrapForModal active={isSearchRouterDisplayed}>
<SearchRouter
onRouterClose={closeSearchRouter}
shouldHideInputCaret={shouldHideInputCaret}
/>
</FocusTrapForModal>
)}
</KeyboardAvoidingView>
</Modal>
);
}
Expand Down