aaron/mattermost-desktop
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

only refresh view when the server URL changes (MM-34565) (#2082) (#2094)

Browse source 
* only refresh view when the server URL changes (MM-34565)

Create views of current and incoming tabs indexed
by unique [URL, TABTYPE] tuples, and diffing them.
Tuples that are identical are recycled, merely
porting the new server name over.

* lint fixes

* WIP

* linting

* remove dependency on by, duad

* provide a more exaplanatory name for TabView.prototype.tuple (urlTypeTuple)

* minor improvements in viewManager

- remove stateful behaviour from makeView
- more descriptive variable names when looping
- create new arrays before sorting (sort is in-place by default)

* resolve linting errors
This commit is contained in:
vas 2022-05-20 18:06:43 +03:00 committed by GitHub
parent d74dba5bad
commit 4efac368d5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 179 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
package-lock.json generated
View file

@ -41,6 +41,7 @@
"@babel/preset-env": "7.16.11",
"@babel/preset-react": "7.16.7",
"@babel/register": "7.17.7",
"@bloomberg/record-tuple-polyfill": "^0.0.4",
"@electron/fuses": "1.5.0",
"@electron/universal": "1.2.1",
"@storybook/addon-actions": "6.4.20",
@ -2081,6 +2082,12 @@
"integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==",
"dev": true
},
"node_modules/@bloomberg/record-tuple-polyfill": {
"version": "0.0.4",
"resolved": "https://registry.npmjs.org/@bloomberg/record-tuple-polyfill/-/record-tuple-polyfill-0.0.4.tgz",
"integrity": "sha512-h0OYmPR3A5Dfbetra/GzxBAzQk8sH7LhRkRUTdagX6nrtlUgJGYCTv4bBK33jsTQw9HDd8PE2x1Ma+iRKEDUsw==",
"dev": true
},
"node_modules/@develar/schema-utils": {
"version": "2.6.5",
"resolved": "https://registry.npmjs.org/@develar/schema-utils/-/schema-utils-2.6.5.tgz",
@ -33023,6 +33030,12 @@
"integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==",
"dev": true
},
"@bloomberg/record-tuple-polyfill": {
"version": "0.0.4",
"resolved": "https://registry.npmjs.org/@bloomberg/record-tuple-polyfill/-/record-tuple-polyfill-0.0.4.tgz",
"integrity": "sha512-h0OYmPR3A5Dfbetra/GzxBAzQk8sH7LhRkRUTdagX6nrtlUgJGYCTv4bBK33jsTQw9HDd8PE2x1Ma+iRKEDUsw==",
"dev": true
},
"@develar/schema-utils": {
"version": "2.6.5",
"resolved": "https://registry.npmjs.org/@develar/schema-utils/-/schema-utils-2.6.5.tgz",

3
package.json
View file

@ -9,7 +9,7 @@
"desktopName": "Mattermost.Desktop",
"homepage": "https://mattermost.com",
"engines": {
"node": ">=4.2.0"
"node": ">=14.6.0"
},
"repository": {
"type": "git",
@ -115,6 +115,7 @@
"@babel/preset-env": "7.16.11",
"@babel/preset-react": "7.16.7",
"@babel/register": "7.17.7",
"@bloomberg/record-tuple-polyfill": "^0.0.4",
"@electron/fuses": "1.5.0",
"@electron/universal": "1.2.1",
"@storybook/addon-actions": "6.4.20",

7
src/common/tabs/BaseTabView.ts
View file

@ -1,9 +1,11 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {Tuple as tuple} from '@bloomberg/record-tuple-polyfill';
import {MattermostServer} from 'common/servers/MattermostServer';
import {getTabViewName, TabType, TabView} from './TabView';
import {getTabViewName, TabType, TabView, TabTuple} from './TabView';
export default abstract class BaseTabView implements TabView {
server: MattermostServer;
@ -14,6 +16,9 @@ export default abstract class BaseTabView implements TabView {
get name(): string {
return getTabViewName(this.server.name, this.type);
}
get urlTypeTuple(): TabTuple {
return tuple(this.url.href, this.type) as TabTuple;
}
get url(): URL {
throw new Error('Not implemented');
}

2
src/common/tabs/TabView.ts
View file

@ -13,6 +13,7 @@ export const TAB_MESSAGING = 'TAB_MESSAGING';
export const TAB_FOCALBOARD = 'TAB_FOCALBOARD';
export const TAB_PLAYBOOKS = 'TAB_PLAYBOOKS';
export type TabType = typeof TAB_MESSAGING | typeof TAB_FOCALBOARD | typeof TAB_PLAYBOOKS;
export type TabTuple = [string, TabType];
export interface TabView {
server: MattermostServer;
@ -21,6 +22,7 @@ export interface TabView {
get type(): TabType;
get url(): URL;
get shouldNotify(): boolean;
get urlTypeTuple(): TabTuple;
}
export function getDefaultTeamWithTabsFromTeam(team: Team) {

6
src/main/views/MattermostView.ts
View file

@ -22,7 +22,7 @@ import {
LOADSCREEN_END,
} from 'common/communication';
import {TabView} from 'common/tabs/TabView';
import {TabView, TabTuple} from 'common/tabs/TabView';
import {ServerInfo} from 'main/server/serverInfo';
import ContextMenu from '../contextMenu';
@ -136,6 +136,10 @@ export class MattermostView extends EventEmitter {
return this.tab.name;
}
get urlTypeTuple(): TabTuple {
return this.tab.urlTypeTuple;
}
resetLoadingStatus = () => {
if (this.status !== Status.LOADING) { // if it's already loading, don't touch anything
delete this.retryLoad;

61
src/main/views/viewManager.test.js
View file

@ -5,6 +5,7 @@
'use strict';
import {dialog} from 'electron';
import {Tuple as tuple} from '@bloomberg/record-tuple-polyfill';
import {BROWSER_HISTORY_PUSH, LOAD_SUCCESS} from 'common/communication';
import {MattermostServer} from 'common/servers/MattermostServer';
@ -29,7 +30,7 @@ jest.mock('electron', () => ({
jest.mock('common/tabs/TabView', () => ({
getServerView: jest.fn(),
getTabViewName: jest.fn(),
getTabViewName: jest.fn((a, b) => `${a}-${b}`),
}));
jest.mock('common/servers/MattermostServer', () => ({
@ -65,15 +66,18 @@ describe('main/views/viewManager', () => {
const viewManager = new ViewManager({});
const onceFn = jest.fn();
const loadFn = jest.fn();
const destroyFn = jest.fn();
beforeEach(() => {
viewManager.createLoadingScreen = jest.fn();
viewManager.showByName = jest.fn();
getServerView.mockImplementation((srv, tab) => ({name: `${srv.name}-${tab.name}`}));
MattermostView.mockImplementation(() => ({
MattermostView.mockImplementation((tab) => ({
on: jest.fn(),
load: loadFn,
once: onceFn,
destroy: destroyFn,
name: tab.name,
}));
});
@ -172,11 +176,31 @@ describe('main/views/viewManager', () => {
viewManager.loadView = jest.fn();
viewManager.showByName = jest.fn();
viewManager.showInitial = jest.fn();
getServerView.mockImplementation((srv, tab) => ({name: `${srv.name}-${tab.name}`, url: new URL(`http://${srv.name}.com`)}));
viewManager.mainWindow.webContents = {
send: jest.fn(),
};
getServerView.mockImplementation((srv, tab) => ({
name: `${srv.name}-${tab.name}`,
urlTypeTuple: tuple(`http://${srv.name}.com/`, tab.name),
url: new URL(`http://${srv.name}.com`),
}));
MattermostServer.mockImplementation((name, url) => ({
name,
url: new URL(url),
}));
const onceFn = jest.fn();
const loadFn = jest.fn();
const destroyFn = jest.fn();
MattermostView.mockImplementation((tab) => ({
on: jest.fn(),
load: loadFn,
once: onceFn,
destroy: destroyFn,
name: tab.name,
urlTypeTuple: tab.urlTypeTuple,
tab,
}));
});
afterEach(() => {
@ -188,13 +212,12 @@ describe('main/views/viewManager', () => {
});
it('should recycle existing views', () => {
const view = {
const makeSpy = jest.spyOn(viewManager, 'makeView');
const view = new MattermostView({
name: 'server1-tab1',
tab: {
name: 'server1-tab1',
url: new URL('http://server1.com'),
},
};
urlTypeTuple: tuple(new URL('http://server1.com').href, 'tab1'),
server: 'server1',
});
viewManager.views.set('server1-tab1', view);
viewManager.reloadConfiguration([
{
@ -210,7 +233,8 @@ describe('main/views/viewManager', () => {
},
]);
expect(viewManager.views.get('server1-tab1')).toBe(view);
expect(viewManager.loadView).not.toHaveBeenCalled();
expect(makeSpy).not.toHaveBeenCalled();
makeSpy.mockRestore();
});
it('should close tabs that arent open', () => {
@ -231,6 +255,7 @@ describe('main/views/viewManager', () => {
});
it('should create new views for new tabs', () => {
const makeSpy = jest.spyOn(viewManager, 'makeView');
viewManager.reloadConfiguration([
{
name: 'server1',
@ -244,13 +269,19 @@ describe('main/views/viewManager', () => {
],
},
]);
expect(viewManager.loadView).toHaveBeenCalledWith({
expect(makeSpy).toHaveBeenCalledWith(
{
name: 'server1',
url: new URL('http://server1.com'),
}, expect.any(Object), {
},
expect.any(Object),
{
name: 'tab1',
isOpen: true,
});
},
'http://server1.com/',
);
makeSpy.mockRestore();
});
it('should set focus to current view on reload', () => {
@ -260,6 +291,8 @@ describe('main/views/viewManager', () => {
name: 'server1-tab1',
url: new URL('http://server1.com'),
},
urlTypeTuple: tuple('http://server1.com/', 'tab1'),
destroy: jest.fn(),
};
viewManager.currentView = 'server1-tab1';
viewManager.views.set('server1-tab1', view);
@ -277,6 +310,7 @@ describe('main/views/viewManager', () => {
},
]);
expect(viewManager.showByName).toHaveBeenCalledWith('server1-tab1');
expect(viewManager.mainWindow.webContents.send).not.toHaveBeenCalled();
});
it('should show initial if currentView has been removed', () => {
@ -286,6 +320,7 @@ describe('main/views/viewManager', () => {
name: 'server1-tab1',
url: new URL('http://server1.com'),
},
urlTypeTuple: ['http://server.com/', 'tab1'],
destroy: jest.fn(),
};
viewManager.currentView = 'server1-tab1';

119
src/main/views/viewManager.ts
View file

@ -3,6 +3,7 @@
import log from 'electron-log';
import {BrowserView, BrowserWindow, dialog, ipcMain, IpcMainEvent} from 'electron';
import {BrowserViewConstructorOptions} from 'electron/main';
import {Tuple as tuple} from '@bloomberg/record-tuple-polyfill';
import {Tab, TeamWithTabs} from 'types/config';
@ -24,7 +25,7 @@ import Config from 'common/config';
import urlUtils from 'common/utils/url';
import Utils from 'common/utils/util';
import {MattermostServer} from 'common/servers/MattermostServer';
import {getServerView, getTabViewName} from 'common/tabs/TabView';
import {getServerView, getTabViewName, TabTuple, TabType} from 'common/tabs/TabView';
import {ServerInfo} from 'main/server/serverInfo';
@ -80,25 +81,34 @@ export class ViewManager {
server.tabs.forEach((tab) => this.loadView(srv, serverInfo, tab));
}
loadView = (srv: MattermostServer, serverInfo: ServerInfo, tab: Tab, url?: string) => {
makeView = (srv: MattermostServer, serverInfo: ServerInfo, tab: Tab, url?: string): MattermostView => {
const tabView = getServerView(srv, tab);
if (!tab.isOpen) {
this.closedViews.set(tabView.name, {srv, tab});
return;
}
if (this.closedViews.has(tabView.name)) {
this.closedViews.delete(tabView.name);
}
const view = new MattermostView(tabView, serverInfo, this.mainWindow, this.viewOptions);
this.views.set(tabView.name, view);
if (!this.loadingScreen) {
this.createLoadingScreen();
}
view.once(LOAD_SUCCESS, this.activateView);
view.load(url);
view.on(UPDATE_TARGET_URL, this.showURLView);
view.on(LOADSCREEN_END, this.finishLoading);
view.on(LOAD_FAILED, this.failLoading);
return view;
}
addView = (view: MattermostView): void => {
this.views.set(view.name, view);
if (this.closedViews.has(view.name)) {
this.closedViews.delete(view.name);
}
if (!this.loadingScreen) {
this.createLoadingScreen();
}
}
loadView = (srv: MattermostServer, serverInfo: ServerInfo, tab: Tab, url?: string) => {
if (!tab.isOpen) {
this.closedViews.set(getTabViewName(srv.name, tab.name), {srv, tab});
return;
}
const view = this.makeView(srv, serverInfo, tab, url);
this.addView(view);
}
reloadViewIfNeeded = (viewName: string) => {
@ -112,32 +122,62 @@ export class ViewManager {
this.configServers.forEach((server) => this.loadServer(server));
}
/** Called when a new configuration is received
* Servers or tabs have been added or edited. We need to
* close, open, or reload tabs, taking care to reuse tabs and
* preserve focus on the currently selected tab. */
reloadConfiguration = (configServers: TeamWithTabs[]) => {
this.configServers = configServers.concat();
const oldviews = this.views;
this.views = new Map();
const sorted = this.configServers.sort((a, b) => a.order - b.order);
let setFocus;
sorted.forEach((server) => {
const srv = new MattermostServer(server.name, server.url);
const serverInfo = new ServerInfo(srv);
server.tabs.forEach((tab) => {
const tabView = getServerView(srv, tab);
const recycle = oldviews.get(tabView.name);
if (recycle && recycle.name === this.currentView) {
setFocus = recycle.name;
const focusedTuple: TabTuple | undefined = this.views.get(this.currentView as string)?.urlTypeTuple;
const current: Map<TabTuple, MattermostView> = new Map();
for (const view of this.views.values()) {
current.set(view.urlTypeTuple, view);
}
const views: Map<TabTuple, MattermostView> = new Map();
const closed: Map<TabTuple, {srv: MattermostServer; tab: Tab; name: string}> = new Map();
const sortedTabs = configServers.flatMap((x) => [...x.tabs].
sort((a, b) => a.order - b.order).
map((t): [TeamWithTabs, Tab] => [x, t]));
for (const [team, tab] of sortedTabs) {
const srv = new MattermostServer(team.name, team.url);
const info = new ServerInfo(srv);
const view = getServerView(srv, tab);
const tabTuple = tuple(new URL(team.url).href, tab.name as TabType);
const recycle = current.get(tabTuple);
if (!tab.isOpen) {
this.closedViews.set(tabView.name, {srv, tab});
} else if (recycle && recycle.tab.name === tabView.name && recycle.tab.url.toString() === urlUtils.parseURL(tabView.url)!.toString()) {
oldviews.delete(recycle.name);
this.views.set(recycle.name, recycle);
closed.set(tabTuple, {srv, tab, name: view.name});
} else if (recycle) {
recycle.serverInfo = info;
recycle.tab.server = srv;
views.set(tabTuple, recycle);
} else {
this.loadView(srv, serverInfo, tab);
views.set(tabTuple, this.makeView(srv, info, tab, tabTuple[0]));
}
});
});
if (this.currentView && (oldviews.has(this.currentView) || this.closedViews.has(this.currentView))) {
}
// commit the data to our local state
// destroy everything that no longer exists
for (const [k, v] of current) {
if (!views.has(k)) {
v.destroy();
}
}
// commit views
this.views = new Map();
for (const x of views.values()) {
this.views.set(x.name, x);
}
// commit closed
for (const x of closed.values()) {
this.closedViews.set(x.name, {srv: x.srv, tab: x.tab});
}
if (focusedTuple && (current.has(focusedTuple) || closed.has(focusedTuple))) {
if (configServers.length) {
delete this.currentView;
this.showInitial();
@ -145,11 +185,12 @@ export class ViewManager {
this.mainWindow.webContents.send(SET_ACTIVE_VIEW);
}
}
oldviews.forEach((unused) => {
unused.destroy();
});
if (setFocus) {
this.showByName(setFocus);
// show the focused tab (or initial)
if (focusedTuple && views.has(focusedTuple)) {
const view = views.get(focusedTuple);
this.currentView = view!.name;
this.showByName(view!.name);
} else {
this.showInitial();
}

16
src/types/external/tuple-record-polyfill.d.ts vendored Normal file
View file

@ -0,0 +1,16 @@
// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
declare module '@bloomberg/record-tuple-polyfill' {
export function Tuple<A>(A): [A];
export function Tuple<A, B>(a: A, b: B): [A, B];
export function Tuple<A, B, C>(a: A, b: B, c: C): [A, B, C];
export function Tuple<A, B, C, D>(a: A, b: B, c: C, d: D): [A, B, C, D];
export function Tuple<A, B, C, D, E>(a: A, b: B, c: C, d: D, e: E): [A, B, C, D, E];
export function Tuple<A, B, C, D, E, F>(a: A, b: B, c: C, d: D, e: E, f: F): [A, B, C, D, E, F];
export function Tuple<A, B, C, D, E, F, G>(a: A, b: B, c: C, d: D, e: E, f: F, g: G): [A, B, C, D, E, F, G];
export function Tuple<A, B, C, D, E, F, G, H>(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H): [A, B, C, D, E, F, G, H];
export function Tuple<A, B, C, D, E, F, G, H, I>(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I): [A, B, C, D, E, F, G, H, I];
export function Tuple<A, B, C, D, E, F, G, H, I, J>(a: A, b: B, c: C, d: D, e: E, f: F, g: G, h: H, i: I, j: J): [A, B, C, D, E, F, G, H, I, J];
export function Record<T>(x: {[key: string]: T}): {[key: string]: T};
}