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

Add notification preference for all the reports and update the design for the group chats #28200

Merged
merged 22 commits into from
Oct 10, 2023
+57 −40
Merged

Add notification preference for all the reports and update the design for the group chats #28200

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
32df987
update the design for group chats
techievivek Sep 25, 2023
78bcb7e
Merge branch 'main' into techievivek_add_notif_pref_all
techievivek Sep 26, 2023
d122a0d
fix profile page
techievivek Sep 27, 2023
c5946e3
updated details page to also count for group chats
techievivek Sep 27, 2023
cf82041
fix the spacing
techievivek Sep 27, 2023
083c924
use prettier
techievivek Sep 27, 2023
994140d
get chat report for profile page
techievivek Sep 27, 2023
5910aa9
Merge branch 'main' into techievivek_add_notif_pref_all
techievivek Sep 29, 2023
d4f11fa
Merge branch 'main' into techievivek_add_notif_pref_all
techievivek Oct 2, 2023
3df0e95
resolve conflicts
techievivek Oct 4, 2023
89fc4ae
revert the double withonyx change
techievivek Oct 4, 2023
5b731c0
Merge branch 'main' into techievivek_add_notif_pref_all
techievivek Oct 4, 2023
491e044
fix the report
techievivek Oct 4, 2023
f050e20
use the display name property
techievivek Oct 4, 2023
d21f21c
get rid of default value
techievivek Oct 4, 2023
3c880c6
don't show notification option if not chatted before
techievivek Oct 4, 2023
79560ff
use getReportName method
techievivek Oct 4, 2023
626ec91
Revert "revert the double withonyx change"
techievivek Oct 5, 2023
4417b5c
fix showing notification preference
techievivek Oct 5, 2023
b8a66ed
use correct property name
techievivek Oct 5, 2023
e2ee8df
Merge branch 'main' into techievivek_add_notif_pref_all
techievivek Oct 6, 2023
ac77a36
resolve conflicts
techievivek Oct 10, 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
17 changes: 2 additions & 15 deletions src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1846,15 +1846,11 @@ function getParentNavigationSubtitle(report) {
function navigateToDetailsPage(report) {
const participantAccountIDs = lodashGet(report, 'participantAccountIDs', []);

if (isChatRoom(report) || isPolicyExpenseChat(report) || isChatThread(report) || isTaskReport(report)) {
Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID));
return;
}
if (participantAccountIDs.length === 1) {
if (isDM(report) && participantAccountIDs.length === 1) {
Navigation.navigate(ROUTES.PROFILE.getRoute(participantAccountIDs[0]));
return;
}
Navigation.navigate(ROUTES.REPORT_PARTICIPANTS.getRoute(report.reportID));
Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS.getRoute(report.reportID));
}

/**
Expand Down Expand Up @@ -3458,14 +3454,6 @@ function getPolicyExpenseChatReportIDByOwner(policyOwner) {
return expenseChat.reportID;
}

/*
* @param {Object|null} report
* @returns {Boolean}
*/
function shouldDisableSettings(report) {
return !isPolicyExpenseChat(report) && !isChatRoom(report) && !isChatThread(report);
}

/**
* @param {Object|null} report
* @param {Object|null} policy - the workspace the report is on, null if the user isn't a member of the workspace
Expand Down Expand Up @@ -3791,7 +3779,6 @@ export {
isDM,
getPolicy,
getPolicyExpenseChatReportIDByOwner,
shouldDisableSettings,
shouldDisableRename,
hasSingleParticipant,
getReportRecipientAccountIDs,
Expand Down
15 changes: 15 additions & 0 deletions src/pages/ProfilePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import variables from '../styles/variables';
import * as ValidationUtils from '../libs/ValidationUtils';
import Permissions from '../libs/Permissions';
import ROUTES from '../ROUTES';
import MenuItemWithTopDescription from '../components/MenuItemWithTopDescription';

const matchType = PropTypes.shape({
params: PropTypes.shape({
Expand Down Expand Up @@ -137,6 +138,11 @@ function ProfilePage(props) {

const chatReportWithCurrentUser = !isCurrentUser && !Session.isAnonymousUser() ? ReportUtils.getChatByParticipants([accountID]) : 0;

const notificationPreference =
chatReportWithCurrentUser.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN
techievivek marked this conversation as resolved.
Show resolved Hide resolved
? props.translate(`notificationPreferencesPage.notificationPreferences.${chatReportWithCurrentUser.notificationPreference}`)
: '';

// eslint-disable-next-line rulesdir/prefer-early-return
useEffect(() => {
if (ValidationUtils.isValidAccountRoute(accountID) && !hasMinimumDetails) {
Expand Down Expand Up @@ -231,6 +237,15 @@ function ProfilePage(props) {
) : null}
{shouldShowLocalTime && <AutoUpdateTime timezone={timezone} />}
</View>
{chatReportWithCurrentUser.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN && (
<MenuItemWithTopDescription
techievivek marked this conversation as resolved.
Show resolved Hide resolved
shouldShowRightIcon
title={notificationPreference}
techievivek marked this conversation as resolved.
Show resolved Hide resolved
description={props.translate('notificationPreferencesPage.label')}
onPress={() => Navigation.navigate(ROUTES.REPORT_SETTINGS_NOTIFICATION_PREFERENCES.getRoute(chatReportWithCurrentUser.reportID))}
wrapperStyle={[styles.mtn6, styles.mb5]}
/>
)}
{!isCurrentUser && !Session.isAnonymousUser() && (
<MenuItem
title={`${props.translate('common.message')}${displayName}`}
Expand Down
37 changes: 19 additions & 18 deletions src/pages/ReportDetailsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ const defaultProps = {
function ReportDetailsPage(props) {
const policy = useMemo(() => props.policies[`${ONYXKEYS.COLLECTION.POLICY}${props.report.policyID}`], [props.policies, props.report.policyID]);
const isPolicyAdmin = useMemo(() => PolicyUtils.isPolicyAdmin(policy), [policy]);
const shouldDisableSettings = useMemo(() => ReportUtils.shouldDisableSettings(props.report), [props.report]);
const shouldUseFullTitle = !shouldDisableSettings || ReportUtils.isTaskReport(props.report);
const shouldUseFullTitle = ReportUtils.isTaskReport(props.report);
Copy link
Contributor

@situchan situchan Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is there reason for removing !shouldDisableSettings || condition here?
This caused regression - #29321

Edit: Ah I think I got answer here

const isChatRoom = useMemo(() => ReportUtils.isChatRoom(props.report), [props.report]);
const isThread = useMemo(() => ReportUtils.isChatThread(props.report), [props.report]);
const isUserCreatedPolicyRoom = useMemo(() => ReportUtils.isUserCreatedPolicyRoom(props.report), [props.report]);
Expand All @@ -72,16 +71,20 @@ function ReportDetailsPage(props) {
const canLeaveRoom = useMemo(() => ReportUtils.canLeaveRoom(props.report, !_.isEmpty(policy)), [policy, props.report]);
const participants = useMemo(() => lodashGet(props.report, 'participantAccountIDs', []), [props.report]);

const isGroupDMChat = useMemo(() => ReportUtils.isDM(props.report) && participants.length > 1, [props.report, participants.length]);
techievivek marked this conversation as resolved.
Show resolved Hide resolved

const menuItems = useMemo(() => {
const items = [
{
const items = [];

if (!isGroupDMChat) {
items.push({
key: CONST.REPORT_DETAILS_MENU_ITEM.SHARE_CODE,
translationKey: 'common.shareCode',
icon: Expensicons.QrCode,
isAnonymousAction: true,
action: () => Navigation.navigate(ROUTES.REPORT_WITH_ID_DETAILS_SHARE_CODE.getRoute(props.report.reportID)),
},
];
});
}

if (isArchivedRoom) {
return items;
Expand All @@ -100,17 +103,15 @@ function ReportDetailsPage(props) {
});
}

if (!shouldDisableSettings) {
items.push({
key: CONST.REPORT_DETAILS_MENU_ITEM.SETTINGS,
translationKey: 'common.settings',
icon: Expensicons.Gear,
isAnonymousAction: false,
action: () => {
Navigation.navigate(ROUTES.REPORT_SETTINGS.getRoute(props.report.reportID));
},
});
}
items.push({
key: CONST.REPORT_DETAILS_MENU_ITEM.SETTINGS,
translationKey: 'common.settings',
icon: Expensicons.Gear,
isAnonymousAction: false,
action: () => {
Navigation.navigate(ROUTES.REPORT_SETTINGS.getRoute(props.report.reportID));
},
});

// Prevent displaying private notes option for threads and task reports
if (!isThread && !ReportUtils.isTaskReport(props.report)) {
Expand All @@ -135,7 +136,7 @@ function ReportDetailsPage(props) {
}

return items;
}, [props.report, participants, isArchivedRoom, shouldDisableSettings, isThread, isUserCreatedPolicyRoom, canLeaveRoom]);
}, [props.report, participants, isArchivedRoom, isThread, isUserCreatedPolicyRoom, canLeaveRoom, isGroupDMChat]);

const displayNamesWithTooltips = useMemo(() => {
const hasMultipleParticipants = participants.length > 1;
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Report/NotificationPreferencePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const propTypes = {
const greenCheckmark = {src: Expensicons.Checkmark, color: themeColors.success};

function NotificationPreferencePage(props) {
const shouldDisableNotificationPreferences = ReportUtils.shouldDisableSettings(props.report) || ReportUtils.isArchivedRoom(props.report);
const shouldDisableNotificationPreferences = ReportUtils.isArchivedRoom(props.report);
const notificationPreferenceOptions = _.map(
_.filter(_.values(CONST.REPORT.NOTIFICATION_PREFERENCE), (pref) => pref !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN),
(preference) => ({
Expand Down
2 changes: 1 addition & 1 deletion src/pages/settings/Report/ReportSettingsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function ReportSettingsPage(props) {
const shouldDisableWelcomeMessage =
ReportUtils.isArchivedRoom(report) || !ReportUtils.isChatRoom(report) || _.isEmpty(linkedWorkspace) || linkedWorkspace.role !== CONST.POLICY.ROLE.ADMIN;

const shouldDisableSettings = _.isEmpty(report) || ReportUtils.shouldDisableSettings(report) || ReportUtils.isArchivedRoom(report);
const shouldDisableSettings = _.isEmpty(report) || ReportUtils.isArchivedRoom(report);
Copy link
Contributor

Choose a reason for hiding this comment

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

After enabling settings for all the reports and revealing access to the notification preference page. We had to test the offline behavior for the newly enabled reports. Coming from #29334 this caused a regression where the creation of a new task missed the optimistic prop notificationPreference.

const shouldShowRoomName = !ReportUtils.isPolicyExpenseChat(report) && !ReportUtils.isChatThread(report);
const notificationPreference =
report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN
Expand Down
4 changes: 4 additions & 0 deletions src/styles/utilities/spacing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ export default {
marginTop: 'auto',
},

mtn6: {
marginTop: -24,
},

mb0: {
marginBottom: 0,
},
Expand Down