Skip to content
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

Fix App does not navigate to last opened public room in small screen devices after Sign in from a public room link #20654

Merged
merged 3 commits into from
Jun 13, 2023
Merged

Fix App does not navigate to last opened public room in small screen devices after Sign in from a public room link #20654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a77531
Fix app does not navigate to last opened public room in small screen …
hoangzinh Jun 13, 2023
81e16d1
Fix eslint
hoangzinh Jun 13, 2023
8b5881c
Update method name
hoangzinh Jun 13, 2023
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
14 changes: 14 additions & 0 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import KeyboardShortcut from '../../KeyboardShortcut';
import Navigation from '../Navigation';
import * as User from '../../actions/User';
import * as Modal from '../../actions/Modal';
import * as Report from '../../actions/Report';
import modalCardStyleInterpolator from './modalCardStyleInterpolator';
import createResponsiveStackNavigator from './createResponsiveStackNavigator';
import SCREENS from '../../../SCREENS';
Expand Down Expand Up @@ -85,13 +86,18 @@ const propTypes = {
session: PropTypes.shape({
email: PropTypes.string.isRequired,
}),

/** The report ID of the last opened public room as anonymous user */
mountiny marked this conversation as resolved.
Show resolved Hide resolved
lastOpenedPublicRoomID: PropTypes.string,

...windowDimensionsPropTypes,
};

const defaultProps = {
session: {
email: null,
},
lastOpenedPublicRoomID: null,
};

class AuthScreens extends React.Component {
Expand All @@ -115,6 +121,11 @@ class AuthScreens extends React.Component {

App.openApp();
App.setUpPoliciesAndNavigate(this.props.session);

if (this.props.lastOpenedPublicRoomID) {
// Re-open the last opened public room if the user logged in from a public room link
Report.openLastOpenedPublicRoom(this.props.lastOpenedPublicRoomID);
}
Download.clearDownloads();
Timing.end(CONST.TIMING.HOMEPAGE_INITIAL_RENDER);

Expand Down Expand Up @@ -274,5 +285,8 @@ export default compose(
session: {
key: ONYXKEYS.SESSION,
},
lastOpenedPublicRoomID: {
key: ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID,
},
}),
)(AuthScreens);
18 changes: 0 additions & 18 deletions src/libs/Navigation/AppNavigator/ReportScreenWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ import reportPropTypes from '../../../pages/reportPropTypes';
import FullScreenLoadingIndicator from '../../../components/FullscreenLoadingIndicator';
import {withNavigationPropTypes} from '../../../components/withNavigation';
import * as App from '../../actions/App';
import * as Report from '../../actions/Report';
import * as Session from '../../actions/Session';

const propTypes = {
/** Available reports that would be displayed in this navigator */
Expand All @@ -33,9 +31,6 @@ const propTypes = {
}),
),

/** The report ID of the last opened public room as anonymous user */
lastOpenedPublicRoomID: PropTypes.string,

isFirstTimeNewExpensifyUser: PropTypes.bool,

/** Navigation route context info provided by react navigation */
Expand All @@ -58,7 +53,6 @@ const defaultProps = {
betas: [],
policies: {},
isFirstTimeNewExpensifyUser: false,
lastOpenedPublicRoomID: null,
};

/**
Expand Down Expand Up @@ -102,15 +96,6 @@ class ReportScreenWrapper extends Component {
}
}

componentDidMount() {
if (!this.props.lastOpenedPublicRoomID || Session.isAnonymousUser()) {
return;
}
// Re-open the last opened public room if the user logged in
Report.setLastOpenedPublicRoom('');
Report.openReport(this.props.lastOpenedPublicRoomID);
}

shouldComponentUpdate(nextProps) {
// Don't update if there is a reportID in the params already
if (lodashGet(this.props.route, 'params.reportID', null)) {
Expand Down Expand Up @@ -161,7 +146,4 @@ export default withOnyx({
isFirstTimeNewExpensifyUser: {
key: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER,
},
lastOpenedPublicRoomID: {
key: ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID,
},
})(ReportScreenWrapper);
13 changes: 13 additions & 0 deletions src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1680,6 +1680,18 @@ function setLastOpenedPublicRoom(reportID) {
Onyx.set(ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID, reportID);
}

/**
* Navigates to the last opened public room
*
* @param {String} lastOpenedPublicRoomID
*/
function openLastOpenedPublicRoom(lastOpenedPublicRoomID) {
Navigation.isNavigationReady().then(() => {
setLastOpenedPublicRoom('');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Shouldn't the value remain the same?

Copy link
Contributor Author

@hoangzinh hoangzinh Jun 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I copied from the old logic. But I think it makes sense if we want it trigger only 1 time. That means if we already visit the public room after sign in, then we refresh the page, it won't trigger again.

Navigation.navigate(ROUTES.getReportRoute(lastOpenedPublicRoomID));
});
}

/**
* Flag a comment as offensive
*
Expand Down Expand Up @@ -1804,4 +1816,5 @@ export {
leaveRoom,
setLastOpenedPublicRoom,
flagComment,
openLastOpenedPublicRoom,
};