Skip to content

fix crash when swiping to chat immediately after creating a room #59207

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.

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
merged 4 commits into from
Apr 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion src/libs/Navigation/OnyxTabNavigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ type OnyxTabNavigatorProps = ChildrenProps & {

/** Whether to show the label when the tab is inactive */
shouldShowLabelWhenInactive?: boolean;

/** Disable swipe between tabs */
disableSwipe?: boolean;
};

// eslint-disable-next-line rulesdir/no-inline-named-export
Expand All @@ -66,6 +69,7 @@ function OnyxTabNavigator({
onTabSelected = () => {},
screenListeners,
shouldShowLabelWhenInactive = true,
disableSwipe = false,
...rest
}: OnyxTabNavigatorProps) {
// Mapping of tab name to focus trap container element
Expand Down Expand Up @@ -137,7 +141,10 @@ function OnyxTabNavigator({
},
...(screenListeners ?? {}),
}}
screenOptions={defaultScreenOptions}
screenOptions={{
...defaultScreenOptions,
swipeEnabled: !disableSwipe,
}}
>
{children}
</TopTab.Navigator>
Expand Down
5 changes: 5 additions & 0 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4547,6 +4547,10 @@ function updateLoadingInitialReportAction(reportID: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {isLoadingInitialReportActions: false});
}

function setNewRoomFormLoading(isLoading = true) {
Onyx.merge(`${ONYXKEYS.FORMS.NEW_ROOM_FORM}`, {isLoading});
}

function clearNewRoomFormError() {
Onyx.set(ONYXKEYS.FORMS.NEW_ROOM_FORM, {
isLoading: false,
Expand Down Expand Up @@ -5346,6 +5350,7 @@ export {
clearGroupChat,
clearIOUError,
clearNewRoomFormError,
setNewRoomFormLoading,
clearPolicyRoomNameErrors,
clearPrivateNotesError,
clearReportFieldKeyErrors,
Expand Down
13 changes: 12 additions & 1 deletion src/pages/NewChatSelectorPage.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import {useNavigation} from '@react-navigation/native';
import React, {useCallback, useMemo, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {useOnyx} from 'react-native-onyx';
import FocusTrapContainerElement from '@components/FocusTrap/FocusTrapContainerElement';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import TabSelector from '@components/TabSelector/TabSelector';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useThemeStyles from '@hooks/useThemeStyles';
import {setNewRoomFormLoading} from '@libs/actions/Report';
import OnyxTabNavigator, {TabScreenWithFocusTrapWrapper, TopTab} from '@libs/Navigation/OnyxTabNavigator';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import NewChatPage from './NewChatPage';
import WorkspaceNewRoomPage from './workspace/WorkspaceNewRoomPage';

Expand All @@ -19,6 +23,8 @@ function NewChatSelectorPage() {
const [headerWithBackBtnContainerElement, setHeaderWithBackButtonContainerElement] = useState<HTMLElement | null>(null);
const [tabBarContainerElement, setTabBarContainerElement] = useState<HTMLElement | null>(null);
const [activeTabContainerElement, setActiveTabContainerElement] = useState<HTMLElement | null>(null);
const [formState] = useOnyx(ONYXKEYS.FORMS.NEW_ROOM_FORM);
const {shouldUseNarrowLayout} = useResponsiveLayout();

// Theoretically, the focus trap container element can be null (due to component unmount/remount), so we filter out the null elements
const containerElements = useMemo(() => {
Expand All @@ -29,6 +35,10 @@ function NewChatSelectorPage() {
setActiveTabContainerElement(activeTabElement ?? null);
}, []);

useEffect(() => {
setNewRoomFormLoading(false);
}, []);

return (
<ScreenWrapper
shouldEnableKeyboardAvoidingView={false}
Expand All @@ -53,6 +63,7 @@ function NewChatSelectorPage() {
tabBar={TabSelector}
onTabBarFocusTrapContainerElementChanged={setTabBarContainerElement}
onActiveTabFocusTrapContainerElementChanged={onTabFocusTrapContainerElementChanged}
disableSwipe={!!formState?.isLoading && shouldUseNarrowLayout}
>
<TopTab.Screen name={CONST.TAB.NEW_CHAT}>
{() => (
Expand Down
13 changes: 9 additions & 4 deletions src/pages/workspace/WorkspaceNewRoomPage.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {useIsFocused} from '@react-navigation/core';
import React, {useCallback, useEffect, useMemo, useState} from 'react';
import {View} from 'react-native';
import {InteractionManager, View} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import BlockingView from '@components/BlockingViews/BlockingView';
Expand Down Expand Up @@ -31,7 +31,7 @@ import {getActivePolicies} from '@libs/PolicyUtils';
import {buildOptimisticChatReport, getCommentLength, getParsedComment, isPolicyAdmin} from '@libs/ReportUtils';
import {isExistingRoomName, isReservedRoomName, isValidRoomNameWithoutLimits} from '@libs/ValidationUtils';
import variables from '@styles/variables';
import {addPolicyReport, clearNewRoomFormError} from '@userActions/Report';
import {addPolicyReport, clearNewRoomFormError, setNewRoomFormLoading} from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -92,6 +92,7 @@ function WorkspaceNewRoomPage() {
* @param values - form input values passed by the Form component
*/
const submit = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.NEW_ROOM_FORM>) => {
setNewRoomFormLoading();
const participants = [session?.accountID ?? CONST.DEFAULT_NUMBER_ID];
const parsedDescription = getParsedComment(values.reportDescription ?? '', {policyID});
const policyReport = buildOptimisticChatReport({
Expand All @@ -106,8 +107,12 @@ function WorkspaceNewRoomPage() {
description: parsedDescription,
});

setNewRoomReportID(policyReport.reportID);
addPolicyReport(policyReport);
InteractionManager.runAfterInteractions(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

@huult Could you explain the changes here? They weren't on your proposal as far as I can tell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee I added this to add a slight delay so we can disable tab switching. Without the delay, we're unable to prevent the tab from being switched.

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee You applied InteractionManager.runAfterInteractions(() => {, and you're still able to swipe?

Copy link
Contributor

@jjcoffee jjcoffee Apr 2, 2025

Choose a reason for hiding this comment

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

@huult Yes, the screen recording is just with your PR's changes. I don't get any crash, though, so I don't know if it's a big deal. The freezing mid-swipe could be an issue though (though pretty minor, since it's a bit of an edge case anyway!).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm weird I can't repro this today 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

This interaction manager causes this issue.
More details in this comment.

requestAnimationFrame(() => {
setNewRoomReportID(policyReport.reportID);
addPolicyReport(policyReport);
});
});
};

useEffect(() => {
Expand Down
Loading