Skip to content

Commit

Permalink
Various fixes (#3287)
Browse files Browse the repository at this point in the history
* Ensure onAppStateChange runs only after GlobalEventHandler is configured (#3268)

* Updated Info.plist with new bluetooth usage description key (#3275)

* Call scrollToIndex only when ref is set and index is in range (#3285)

* Update ios/Mattermost/Info.plist

Co-Authored-By: Elias Nahum <[email protected]>

* Null check on current (#3309)

* Null check on current

* Null check on current

* [MM-18779] Reset moment local on logout (#3310)

* Reset moment local on logout

* Update app/selectors/i18n.js

Co-Authored-By: Elias Nahum <[email protected]>
  • Loading branch information
migbot and enahum authored Sep 25, 2019
1 parent 20a2078 commit 290e65a
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 30 deletions.
31 changes: 17 additions & 14 deletions app/components/post_list/post_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,30 +299,33 @@ export default class PostList extends PureComponent {

scrollToBottom = () => {
setTimeout(() => {
if (this.flatListRef && this.flatListRef.current) {
if (this.flatListRef.current) {
this.flatListRef.current.scrollToOffset({offset: 0, animated: true});
}
}, 250);
};

flatListScrollToIndex = (index) => {
this.flatListRef.current.scrollToIndex({
animated: false,
index,
viewOffset: 0,
viewPosition: 1, // 0 is at bottom
});
}

scrollToIndex = (index) => {
if (this.flatListRef?.current) {
this.animationFrameInitialIndex = requestAnimationFrame(() => {
this.flatListRef.current.scrollToIndex({
animated: false,
index,
viewOffset: 0,
viewPosition: 1, // 0 is at bottom
});
});
}
this.animationFrameInitialIndex = requestAnimationFrame(() => {
if (this.flatListRef.current && index > 0 && index <= this.getItemCount()) {
this.flatListScrollToIndex(index);
}
});
};

scrollToInitialIndexIfNeeded = (index, count = 0) => {
if (!this.hasDoneInitialScroll && this.flatListRef?.current) {
this.hasDoneInitialScroll = true;

if (!this.hasDoneInitialScroll) {
if (index > 0 && index <= this.getItemCount()) {
this.hasDoneInitialScroll = true;
this.scrollToIndex(index);
} else if (count < 3) {
setTimeout(() => {
Expand Down
28 changes: 28 additions & 0 deletions app/components/post_list/post_list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,32 @@ describe('PostList', () => {
expect(baseProps.actions.handleSelectChannelByName).toHaveBeenCalled();
expect(wrapper.getElement()).toMatchSnapshot();
});

test('should call flatListScrollToIndex only when ref is set and index is in range', () => {
jest.spyOn(global, 'requestAnimationFrame').mockImplementation((cb) => cb());

const instance = wrapper.instance();
const flatListScrollToIndex = jest.spyOn(instance, 'flatListScrollToIndex');
const indexInRange = baseProps.postIds.length;
const indexOutOfRange = [-1, indexInRange + 1];

instance.flatListRef = {
current: null,
};
instance.scrollToIndex(indexInRange);
expect(flatListScrollToIndex).not.toHaveBeenCalled();

instance.flatListRef = {
current: {
scrollToIndex: jest.fn(),
},
};
for (const index of indexOutOfRange) {
instance.scrollToIndex(index);
expect(flatListScrollToIndex).not.toHaveBeenCalled();
}

instance.scrollToIndex(indexInRange);
expect(flatListScrollToIndex).toHaveBeenCalled();
});
});
10 changes: 2 additions & 8 deletions app/i18n/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,21 +108,15 @@ function loadTranslation(locale) {
}
}

let momentLocale = DEFAULT_LOCALE;

function setMomentLocale(locale) {
if (momentLocale !== locale) {
momentLocale = moment.locale(locale);
}
export function resetMomentLocale() {
moment.locale(DEFAULT_LOCALE);
}

export function getTranslations(locale) {
if (!TRANSLATIONS[locale]) {
loadTranslation(locale);
}

setMomentLocale(locale.toLowerCase());

return TRANSLATIONS[locale] || TRANSLATIONS[DEFAULT_LOCALE];
}

Expand Down
18 changes: 11 additions & 7 deletions app/init/global_event_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {selectDefaultChannel} from 'app/actions/views/channel';
import {showOverlay} from 'app/actions/navigation';
import {loadConfigAndLicense, setDeepLinkURL, startDataCleanup} from 'app/actions/views/root';
import {NavigationTypes, ViewTypes} from 'app/constants';
import {getTranslations} from 'app/i18n';
import {getTranslations, resetMomentLocale} from 'app/i18n';
import mattermostManaged from 'app/mattermost_managed';
import PushNotifications from 'app/push_notifications';
import {getCurrentLocale} from 'app/selectors/i18n';
Expand All @@ -40,7 +40,6 @@ class GlobalEventHandler {
EventEmitter.on(General.SERVER_VERSION_CHANGED, this.onServerVersionChanged);
EventEmitter.on(General.CONFIG_CHANGED, this.onServerConfigChanged);
EventEmitter.on(General.SWITCH_TO_DEFAULT_CHANNEL, this.onSwitchToDefaultChannel);
this.turnOnInAppNotificationHandling();
Dimensions.addEventListener('change', this.onOrientationChange);
AppState.addEventListener('change', this.onAppStateChange);
Linking.addEventListener('url', this.onDeepLink);
Expand Down Expand Up @@ -77,6 +76,10 @@ class GlobalEventHandler {
this.store = opts.store;
this.launchApp = opts.launchApp;

// onAppStateChange may be called by the AppState listener before we
// configure the global event handler so we manually call it here
this.onAppStateChange('active');

const window = Dimensions.get('window');
this.onOrientationChange({window});

Expand Down Expand Up @@ -113,12 +116,12 @@ class GlobalEventHandler {

if (this.store) {
this.store.dispatch(setAppState(isActive));
}

if (isActive && emmProvider.previousAppState === 'background') {
this.appActive();
} else if (isBackground) {
this.appInactive();
if (isActive && (!emmProvider.enabled || emmProvider.previousAppState === 'background')) {
this.appActive();
} else if (isBackground) {
this.appInactive();
}
}

emmProvider.previousAppState = appState;
Expand All @@ -142,6 +145,7 @@ class GlobalEventHandler {
this.store.dispatch(setServerVersion(''));
deleteFileCache();
removeAppCredentials();
resetMomentLocale();

PushNotifications.clearNotifications();

Expand Down
53 changes: 52 additions & 1 deletion app/init/global_event_handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import thunk from 'redux-thunk';

import intitialState from 'app/initial_state';
import PushNotification from 'app/push_notifications';
import * as I18n from 'app/i18n';

import GlobalEventHandler from './global_event_handler';

Expand All @@ -15,6 +16,12 @@ jest.mock('app/init/credentials', () => ({
removeAppCredentials: jest.fn(),
}));

jest.mock('app/utils/error_handling', () => ({
default: {
initializeErrorHandling: jest.fn(),
},
}));

jest.mock('react-native-notifications', () => ({
addEventListener: jest.fn(),
cancelAllLocalNotifications: jest.fn(),
Expand All @@ -29,10 +36,54 @@ GlobalEventHandler.store = store;

// TODO: Add Android test as part of https://mattermost.atlassian.net/browse/MM-17110
describe('GlobalEventHandler', () => {
it('should clear notifications on logout', async () => {
it('should clear notifications and reset moment locale on logout', async () => {
const clearNotifications = jest.spyOn(PushNotification, 'clearNotifications');
const resetMomentLocale = jest.spyOn(I18n, 'resetMomentLocale');

await GlobalEventHandler.onLogout();
expect(clearNotifications).toHaveBeenCalled();
expect(resetMomentLocale).toHaveBeenCalledWith();
});

it('should call onAppStateChange after configuration', () => {
const onAppStateChange = jest.spyOn(GlobalEventHandler, 'onAppStateChange');

GlobalEventHandler.configure({store});
expect(GlobalEventHandler.store).not.toBeNull();
expect(onAppStateChange).toHaveBeenCalledWith('active');
});

it('should handle onAppStateChange to active if the store set', () => {
const appActive = jest.spyOn(GlobalEventHandler, 'appActive');
const appInactive = jest.spyOn(GlobalEventHandler, 'appInactive');
expect(GlobalEventHandler.store).not.toBeNull();

GlobalEventHandler.onAppStateChange('active');
expect(appActive).toHaveBeenCalled();
expect(appInactive).not.toHaveBeenCalled();
});

it('should handle onAppStateChange to background if the store set', () => {
const appActive = jest.spyOn(GlobalEventHandler, 'appActive');
const appInactive = jest.spyOn(GlobalEventHandler, 'appInactive');
expect(GlobalEventHandler.store).not.toBeNull();

GlobalEventHandler.onAppStateChange('background');
expect(appActive).not.toHaveBeenCalled();
expect(appInactive).toHaveBeenCalled();
});

it('should not handle onAppStateChange if the store is not set', () => {
const appActive = jest.spyOn(GlobalEventHandler, 'appActive');
const appInactive = jest.spyOn(GlobalEventHandler, 'appInactive');
GlobalEventHandler.store = null;

GlobalEventHandler.onAppStateChange('active');
expect(appActive).not.toHaveBeenCalled();
expect(appInactive).not.toHaveBeenCalled();

GlobalEventHandler.onAppStateChange('background');
expect(appActive).not.toHaveBeenCalled();
expect(appInactive).not.toHaveBeenCalled();
});
});
2 changes: 2 additions & 0 deletions ios/Mattermost/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
</dict>
<key>NSAppleMusicUsageDescription</key>
<string>Let Mattermost access your Media files</string>
<key>NSBluetoothAlwaysUsageDescription</key>
<string>Share post data <span class="x x-first x-last">across</span> devices with Mattermost</string>
<key>NSBluetoothPeripheralUsageDescription</key>
<string>Share post data accross devices with Mattermost</string>
<key>NSCalendarsUsageDescription</key>
Expand Down
3 changes: 3 additions & 0 deletions test/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ jest.mock('NativeModules', () => {
addEventListener: jest.fn(),
getCurrentState: jest.fn().mockResolvedValue({isConnected: true}),
},
StatusBarManager: {
getHeight: jest.fn(),
},
};
});
jest.mock('NativeEventEmitter');
Expand Down

0 comments on commit 290e65a

Please sign in to comment.