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.

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 the unread count not going away when you've signed out #13600

Merged
merged 15 commits into from
Dec 30, 2022
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
"react-native-image-picker": "^4.10.2",
"react-native-image-size": "git+https://github.com/Expensify/react-native-image-size#6b5ab5110dc3ed554f8eafbc38d7d87c17147972",
"react-native-modal": "^13.0.0",
"react-native-onyx": "1.0.29",
"react-native-onyx": "1.0.31",
"react-native-pdf": "^6.6.2",
"react-native-performance": "^2.0.0",
"react-native-permissions": "^3.0.1",
Expand Down
4 changes: 4 additions & 0 deletions src/Expensify.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import * as User from './libs/actions/User';
import NetworkConnection from './libs/NetworkConnection';
import Navigation from './libs/Navigation/Navigation';

// This lib needs to be imported, but it has nothing to export since all it contains is an Onyx connection
// eslint-disable-next-line no-unused-vars
import UnreadIndicatorUpdater from './libs/UnreadIndicatorUpdater';

Onyx.registerLogger(({level, message}) => {
if (level === 'alert') {
Log.alert(message);
Expand Down
3 changes: 0 additions & 3 deletions src/libs/Navigation/AppNavigator/AuthScreens.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import compose from '../../compose';
import * as PersonalDetails from '../../actions/PersonalDetails';
import * as Pusher from '../../Pusher/pusher';
import PusherConnectionManager from '../../PusherConnectionManager';
import UnreadIndicatorUpdater from '../../UnreadIndicatorUpdater';
import ROUTES from '../../../ROUTES';

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.

OK, sorry. I think I might have broken this when I just fixed the linter tests via the GitHub web interface. Sorry about that! Let me do this properly and test it out again.

Copy link
Contributor

Choose a reason for hiding this comment

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

changes looks good

import ONYXKEYS from '../../../ONYXKEYS';
import Timing from '../../actions/Timing';
Expand Down Expand Up @@ -101,8 +100,6 @@ class AuthScreens extends React.Component {
User.subscribeToUserEvents();
});

// Listen for report changes and fetch some data we need on initialization
UnreadIndicatorUpdater.listenForReportChanges();
App.openApp();
App.setUpPoliciesAndNavigate(this.props.session);
Timing.end(CONST.TIMING.HOMEPAGE_INITIAL_RENDER);
Expand Down
2 changes: 0 additions & 2 deletions src/libs/Navigation/NavigationRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import AppNavigator from './AppNavigator';
import FullScreenLoadingIndicator from '../../components/FullscreenLoadingIndicator';
import themeColors from '../../styles/themes/default';
import styles from '../../styles/styles';
import UnreadIndicatorUpdater from '../UnreadIndicatorUpdater';
import Log from '../Log';

// https://reactnavigation.org/docs/themes
Expand Down Expand Up @@ -46,7 +45,6 @@ function parseAndLogRoute(state) {
Log.info('Navigating to route', false, {path: currentPath});
}

UnreadIndicatorUpdater.throttledUpdatePageTitleAndUnreadCount();
Navigation.setIsNavigationReady();
}

Expand Down
4 changes: 4 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,10 @@ function buildOptimisticWorkspaceChats(policyID, policyName) {
* @returns {Boolean}
*/
function isUnread(report) {
if (!report) {
return false;
}

const lastReadSequenceNumber = report.lastReadSequenceNumber || 0;
const maxSequenceNumber = report.maxSequenceNumber || 0;
return lastReadSequenceNumber < maxSequenceNumber;
Expand Down
58 changes: 9 additions & 49 deletions src/libs/UnreadIndicatorUpdater/index.js
Original file line number Diff line number Diff line change
@@ -1,54 +1,14 @@
import _ from 'underscore';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '../../ONYXKEYS';
import updateUnread from './updateUnread';
import updateUnread from './updateUnread/index';
import * as ReportUtils from '../ReportUtils';

const reports = {};

/**
* Updates the title and favicon of the current browser tab and Mac OS or iOS dock icon with an unread indicator.
* Note: We are throttling this since listening for report changes can trigger many updates depending on how many reports
* a user has and how often they are updated.
*/
const throttledUpdatePageTitleAndUnreadCount = _.throttle(() => {
const totalCount = _.filter(reports, ReportUtils.isUnread).length;
updateUnread(totalCount);
}, 100, {leading: false});

let connectionID;

/**
* Bind to the report collection key and update
* the title and unread count indicators
*/
function listenForReportChanges() {
connectionID = Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
callback: (report) => {
if (!report || !report.reportID) {
return;
}

reports[report.reportID] = report;
throttledUpdatePageTitleAndUnreadCount();
},
});
}

/**
* Remove the subscription callback when we no longer need it.
*/
function stopListeningForReportChanges() {
if (!connectionID) {
return;
}

Onyx.disconnect(connectionID);
}

export default {
listenForReportChanges,
stopListeningForReportChanges,
throttledUpdatePageTitleAndUnreadCount,
};
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (reportsFromOnyx) => {
const unreadReports = _.filter(reportsFromOnyx, ReportUtils.isUnread);
updateUnread(_.size(unreadReports));
},
});
14 changes: 9 additions & 5 deletions src/libs/UnreadIndicatorUpdater/updateUnread/index.website.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ import CONFIG from '../../../CONFIG';
function updateUnread(totalCount) {
const hasUnread = totalCount !== 0;

// There is a Chrome browser bug that causes the title to revert back to the previous when we are navigating back. Setting the title to an empty string
// seems to improve this issue.
document.title = '';
document.title = hasUnread ? `(${totalCount}) ${CONFIG.SITE_TITLE}` : CONFIG.SITE_TITLE;
document.getElementById('favicon').href = hasUnread ? CONFIG.FAVICON.UNREAD : CONFIG.FAVICON.DEFAULT;
// This setTimeout is required because due to how react rendering messes with the DOM, the document title can't be modified synchronously, and we must wait until all JS is done
// running before setting the title.
setTimeout(() => {
// There is a Chrome browser bug that causes the title to revert back to the previous when we are navigating back. Setting the title to an empty string
// seems to improve this issue.
document.title = '';
document.title = hasUnread ? `(${totalCount}) ${CONFIG.SITE_TITLE}` : CONFIG.SITE_TITLE;
document.getElementById('favicon').href = hasUnread ? CONFIG.FAVICON.UNREAD : CONFIG.FAVICON.DEFAULT;
}, 0);
}

export default updateUnread;
3 changes: 0 additions & 3 deletions src/libs/actions/Session/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import PushNotification from '../../Notification/PushNotification';
import Timing from '../Timing';
import CONST from '../../../CONST';
import * as Localize from '../../Localize';
import UnreadIndicatorUpdater from '../../UnreadIndicatorUpdater';
import Timers from '../../Timers';
import * as Pusher from '../../Pusher/pusher';
import * as User from '../User';
Expand Down Expand Up @@ -369,8 +368,6 @@ function clearSignInData() {
* Put any logic that needs to run when we are signed out here. This can be triggered when the current tab or another tab signs out.
*/
function cleanupSession() {
// We got signed out in this tab or another so clean up any subscriptions and timers
UnreadIndicatorUpdater.stopListeningForReportChanges();
PushNotification.deregister();
PushNotification.clearNotifications();
Pusher.disconnect();
Expand Down
4 changes: 0 additions & 4 deletions src/pages/signin/SignInPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import ONYXKEYS from '../../ONYXKEYS';
import styles from '../../styles/styles';
import updateUnread from '../../libs/UnreadIndicatorUpdater/updateUnread/index';
import compose from '../../libs/compose';
import SignInPageLayout from './SignInPageLayout';
import LoginForm from './LoginForm';
Expand Down Expand Up @@ -44,9 +43,6 @@ const defaultProps = {

class SignInPage extends Component {
componentDidMount() {
// Always reset the unread counter to zero on this page
// NOTE: We need to wait for the next tick to ensure that the unread indicator is updated
setTimeout(() => updateUnread(0), 0);
Performance.measureTTI();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/UnreadIndicatorsTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function signInAndGetAppWithUnreadChat() {
}

describe('Unread Indicators', () => {
afterEach(Onyx.clear);
afterEach(() => Onyx.clear());

it('Display bold in the LHN for unread chat and new line indicator above the chat message when we navigate to it', () => {
let renderedApp;
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/NetworkTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ beforeEach(() => {
// Wait for any Log command to finish and Onyx to fully clear
jest.advanceTimersByTime(CONST.NETWORK.PROCESS_REQUEST_DELAY_MS);
return waitForPromisesToResolve()
.then(Onyx.clear)
.then(() => Onyx.clear())
.then(waitForPromisesToResolve);
});

Expand Down