Skip to content

fix: Room - No animation when returning to room settings after saving new room description. #54623

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
jasperhuangg merged 4 commits into from
Jan 8, 2025
Merged

fix: Room - No animation when returning to room settings after saving new room description. #54623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3af8b5
fix: Room - No animation when returning to room settings after saving…
Krishna2323 Dec 27, 2024
99f1873
fix eslint.
Krishna2323 Dec 27, 2024
4fcb112
fix eslint.
Krishna2323 Dec 27, 2024
1cbc988
migrate GroupChatNameEditPage to use useOnyx.
Krishna2323 Dec 27, 2024
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
4 changes: 2 additions & 2 deletions src/libs/ValidationUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,8 @@ function isReservedRoomName(roomName: string): boolean {
/**
* Checks if the room name already exists.
*/
function isExistingRoomName(roomName: string, reports: OnyxCollection<Report>, policyID: string): boolean {
return Object.values(reports ?? {}).some((report) => report && report.policyID === policyID && report.reportName === roomName);
function isExistingRoomName(roomName: string, reports: OnyxCollection<Report>, policyID: string | undefined): boolean {
return Object.values(reports ?? {}).some((report) => report && policyID && report.policyID === policyID && report.reportName === roomName);
}

/**
Expand Down
30 changes: 11 additions & 19 deletions src/pages/GroupChatNameEditPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React, {useCallback, useMemo} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
import type {FormOnyxValues} from '@components/Form/types';
Expand All @@ -22,23 +21,18 @@ import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import INPUT_IDS from '@src/types/form/NewChatNameForm';
import type {Report as ReportOnyxType} from '@src/types/onyx';
import type NewGroupChatDraft from '@src/types/onyx/NewGroupChatDraft';
import type {Errors} from '@src/types/onyx/OnyxCommon';

type GroupChatNameEditPageOnyxProps = {
groupChatDraft: OnyxEntry<NewGroupChatDraft>;
type GroupChatNameEditPageProps = Partial<PlatformStackScreenProps<NewChatNavigatorParamList, typeof SCREENS.NEW_CHAT.NEW_CHAT_EDIT_NAME>> & {
report?: ReportOnyxType;
};

type GroupChatNameEditPageProps = GroupChatNameEditPageOnyxProps &
Partial<PlatformStackScreenProps<NewChatNavigatorParamList, typeof SCREENS.NEW_CHAT.NEW_CHAT_EDIT_NAME>> & {
report?: ReportOnyxType;
};

function GroupChatNameEditPage({groupChatDraft, report}: GroupChatNameEditPageProps) {
function GroupChatNameEditPage({report}: GroupChatNameEditPageProps) {
// If we have a report this means we are using this page to update an existing Group Chat name
// In this case its better to use empty string as the reportID if there is no reportID
const reportID = report?.reportID ?? '';
const reportID = report?.reportID;
const isUpdatingExistingReport = !!reportID;
const [groupChatDraft] = useOnyx(ONYXKEYS.NEW_GROUP_CHAT_DRAFT);

@c3024 c3024 Apr 10, 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.

useOnyx does not delay the page till this value is loaded. So, if this page is refreshed, the initial value is undefined and the default value for the input becomes an unexpected value. More details here #58967


const styles = useThemeStyles();
const {translate} = useLocalize();
Expand Down Expand Up @@ -69,13 +63,15 @@ function GroupChatNameEditPage({groupChatDraft, report}: GroupChatNameEditPagePr
if (values[INPUT_IDS.NEW_CHAT_NAME] !== currentChatName) {
Report.updateGroupChatName(reportID, values[INPUT_IDS.NEW_CHAT_NAME] ?? '');
}
Navigation.goBack(ROUTES.REPORT_SETTINGS.getRoute(reportID));

Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.REPORT_SETTINGS.getRoute(reportID)));

return;
}
if (values[INPUT_IDS.NEW_CHAT_NAME] !== currentChatName) {
Report.setGroupDraft({reportName: values[INPUT_IDS.NEW_CHAT_NAME]});
}
Navigation.goBack(ROUTES.NEW_CHAT_CONFIRM);
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.NEW_CHAT_CONFIRM));
},
[isUpdatingExistingReport, reportID, currentChatName],
);
Expand Down Expand Up @@ -117,8 +113,4 @@ function GroupChatNameEditPage({groupChatDraft, report}: GroupChatNameEditPagePr

GroupChatNameEditPage.displayName = 'GroupChatNameEditPage';

export default withOnyx<GroupChatNameEditPageProps, GroupChatNameEditPageOnyxProps>({
groupChatDraft: {
key: ONYXKEYS.NEW_GROUP_CHAT_DRAFT,
},
})(GroupChatNameEditPage);
export default GroupChatNameEditPage;
2 changes: 1 addition & 1 deletion src/pages/PrivateNotes/PrivateNotesEditPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ function PrivateNotesEditPage({route, report, accountID}: PrivateNotesEditPagePr
if (!Object.values<Note>({...report.privateNotes, [route.params.accountID]: {note: editedNote}}).some((item) => item.note)) {
ReportUtils.navigateToDetailsPage(report, backTo);
} else {
Navigation.goBack(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID, backTo));
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.PRIVATE_NOTES_LIST.getRoute(report.reportID, backTo)));
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/pages/RoomDescriptionPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function RoomDescriptionPage({report, policies}: RoomDescriptionPageProps) {
}, []);

const goBack = useCallback(() => {
Navigation.goBack(backTo ?? ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID));
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(backTo ?? ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID)));
}, [report.reportID, backTo]);

const submitForm = useCallback(() => {
Expand Down
25 changes: 8 additions & 17 deletions src/pages/settings/Report/RoomNamePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import {useIsFocused, useRoute} from '@react-navigation/native';
import React, {useCallback, useRef} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxCollection} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
Expand All @@ -27,25 +26,21 @@ import type SCREENS from '@src/SCREENS';
import INPUT_IDS from '@src/types/form/RoomNameForm';
import type {Report} from '@src/types/onyx';

type RoomNamePageOnyxProps = {
/** All reports shared with the user */
reports: OnyxCollection<Report>;
};

type RoomNamePageProps = RoomNamePageOnyxProps & {
type RoomNamePageProps = {
report: Report;
};

function RoomNamePage({report, reports}: RoomNamePageProps) {
function RoomNamePage({report}: RoomNamePageProps) {
const route = useRoute<PlatformStackRouteProp<ReportSettingsNavigatorParamList, typeof SCREENS.REPORT_SETTINGS.NAME>>();
const styles = useThemeStyles();
const roomNameInputRef = useRef<AnimatedTextInputRef>(null);
const isFocused = useIsFocused();
const {translate} = useLocalize();
const reportID = report?.reportID ?? '-1';
const reportID = report?.reportID;
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);

const goBack = useCallback(() => {
Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID, route.params.backTo));
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(reportID, route.params.backTo)));
}, [reportID, route.params.backTo]);

const validate = useCallback(
Expand All @@ -66,7 +61,7 @@ function RoomNamePage({report, reports}: RoomNamePageProps) {
} else if (ValidationUtils.isReservedRoomName(values.roomName)) {
// Certain names are reserved for default rooms and should not be used for policy rooms.
ErrorUtils.addErrorMessage(errors, 'roomName', translate('newRoomPage.roomNameReservedError', {reservedName: values.roomName}));
} else if (ValidationUtils.isExistingRoomName(values.roomName, reports, report?.policyID ?? '-1')) {
} else if (ValidationUtils.isExistingRoomName(values.roomName, reports, report?.policyID)) {
// The room name can't be set to one that already exists on the policy
ErrorUtils.addErrorMessage(errors, 'roomName', translate('newRoomPage.roomAlreadyExistsError'));
} else if (values.roomName.length > CONST.TITLE_CHARACTER_LIMIT) {
Expand Down Expand Up @@ -122,8 +117,4 @@ function RoomNamePage({report, reports}: RoomNamePageProps) {

RoomNamePage.displayName = 'RoomNamePage';

export default withOnyx<RoomNamePageProps, RoomNamePageOnyxProps>({
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
},
})(RoomNamePage);
export default RoomNamePage;
2 changes: 1 addition & 1 deletion src/pages/workspace/WorkspaceNamePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function WorkspaceNamePage({policy}: Props) {

Policy.updateGeneralSettings(policy.id, values.name.trim(), policy.outputCurrency);
Keyboard.dismiss();
Navigation.goBack();
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack());
},
[policy],
);
Expand Down
4 changes: 2 additions & 2 deletions src/pages/workspace/WorkspaceProfileDescriptionPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ function WorkspaceProfileDescriptionPage({policy}: Props) {

Policy.updateWorkspaceDescription(policy.id, values.description.trim(), policy.description ?? '');
Keyboard.dismiss();
Navigation.goBack();
Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.goBack());
},
[policy],
);

return (
<AccessOrNotFoundWrapper
policyID={policy?.id ?? '-1'}
policyID={policy?.id}

@Pujan92 Pujan92 Dec 30, 2024

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.

Why are we applying changes that don't look related to our issue?

@Krishna2323 Krishna2323 Jan 5, 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.

https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#default-value-for-inexistent-ids

I updated it because now we can't use default string values as fallback, please refer to the above link.

accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN]}
>
<ScreenWrapper
Expand Down