Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 11 additions & 2 deletions src/app/PersistableValues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,13 @@ type PersistableValues_4_15_0 = PersistableValues_4_14_0 & {
e2ePdfPreviewSizeLimit: number;
};

type PersistableValues_4_16_0 = PersistableValues_4_15_0 & {
navigationLayout: 'tabs' | 'sidebar';
};

export type PersistableValues = Pick<
PersistableValues_4_15_0,
keyof PersistableValues_4_15_0
PersistableValues_4_16_0,
keyof PersistableValues_4_16_0
>;

export const migrations = {
Expand Down Expand Up @@ -233,4 +237,9 @@ export const migrations = {
...before,
e2ePdfPreviewSizeLimit: DEFAULT_E2E_PDF_PREVIEW_SIZE_LIMIT_MB,
}),
'>=4.16.0': (before: PersistableValues_4_15_0): PersistableValues_4_16_0 => ({
...before,
navigationLayout:
(before as Partial<PersistableValues_4_16_0>).navigationLayout ?? 'tabs',
}),
};
20 changes: 20 additions & 0 deletions src/app/__tests__/PersistableValues.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,24 @@ describe('PersistableValues migrations', () => {
},
});
});

it('preserves a persisted navigationLayout value', () => {
const before = {
navigationLayout: 'sidebar',
} as unknown as Parameters<(typeof migrations)['>=4.16.0']>[0];

expect(migrations['>=4.16.0'](before)).toEqual(
expect.objectContaining({ navigationLayout: 'sidebar' })
);
});

it('defaults navigationLayout to tabs when absent', () => {
const before = {} as unknown as Parameters<
(typeof migrations)['>=4.16.0']
>[0];

expect(migrations['>=4.16.0'](before)).toEqual(
expect.objectContaining({ navigationLayout: 'tabs' })
);
});
});
119 changes: 78 additions & 41 deletions src/app/main/data.spec.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,48 @@
import * as store from '../../store';
import { APP_SETTINGS_LOADED } from '../actions';
import { mergePersistableValues } from './data';
import { getPersistedValues } from './persistence';

jest.mock('../../store');

jest.mock('./persistence', () => ({
getPersistedValues: jest.fn().mockReturnValue({}),
persistValues: jest.fn(),
}));

jest.mock('fs', () => ({
promises: {
readFile: jest.fn().mockRejectedValue(new Error('File not found')),
unlink: jest.fn().mockResolvedValue(undefined),
},
}));

jest.mock('electron', () => ({
app: {
getPath: jest.fn().mockReturnValue('/user/data'),
getVersion: jest.fn().mockReturnValue('0.0.0'),
},
}));

jest.mock('../../logging', () => ({
logger: { warn: jest.fn(), error: jest.fn(), info: jest.fn() },
}));

const mockDispatch = jest.fn();
const mockSelect = jest.fn();

beforeEach(() => {
jest.clearAllMocks();
(store.dispatch as jest.Mock).mockImplementation(mockDispatch);
(store.select as jest.Mock).mockImplementation(mockSelect);
(getPersistedValues as jest.Mock).mockReturnValue({});
});

describe('mergePersistableValues', () => {
const mockInitialValues = {
isMenuBarEnabled: true,
isSideBarEnabled: true,
navigationLayout: 'tabs' as const,
rootWindowState: {
focused: true,
visible: true,
Expand All @@ -30,73 +56,79 @@ describe('mergePersistableValues', () => {

beforeEach(() => {
mockSelect.mockReturnValue(mockInitialValues);

jest.doMock('./persistence', () => ({
getPersistedValues: jest.fn().mockReturnValue({}),
}));

jest.doMock('fs', () => ({
promises: {
readFile: jest.fn().mockRejectedValue(new Error('File not found')),
unlink: jest.fn().mockResolvedValue(undefined),
},
}));

jest.doMock('electron', () => ({
app: {
getPath: jest.fn().mockReturnValue('/user/data'),
},
}));
});

describe('menubar and sidebar recovery mechanism', () => {
it('should enable sidebar when both menubar and sidebar are disabled', async () => {
describe('menubar recovery mechanism', () => {
const originalPlatform = process.platform;

afterEach(() => {
Object.defineProperty(process, 'platform', {
value: originalPlatform,
writable: true,
configurable: true,
});
});

it('should enable menubar on Linux when in tabs layout with menubar disabled', async () => {
Object.defineProperty(process, 'platform', {
value: 'linux',
writable: true,
configurable: true,
});
const localStorage = {};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: false,
isSideBarEnabled: false,
navigationLayout: 'tabs',
});

await mergePersistableValues(localStorage);

expect(mockDispatch).toHaveBeenCalledWith({
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: false,
isSideBarEnabled: true,
isMenuBarEnabled: true,
}),
});
});

it('should not modify settings when menubar is enabled and sidebar is disabled', async () => {
it('should not modify settings on Linux when in sidebar layout with menubar disabled', async () => {
Object.defineProperty(process, 'platform', {
value: 'linux',
writable: true,
configurable: true,
});
const localStorage = {};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: true,
isSideBarEnabled: false,
isMenuBarEnabled: false,
navigationLayout: 'sidebar',
});

await mergePersistableValues(localStorage);

expect(mockDispatch).toHaveBeenCalledWith({
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: true,
isSideBarEnabled: false,
isMenuBarEnabled: false,
}),
});
});

it('should not modify settings when sidebar is enabled and menubar is disabled', async () => {
it('should not modify settings on non-Linux platforms when in tabs layout with menubar disabled', async () => {
Object.defineProperty(process, 'platform', {
value: 'darwin',
writable: true,
configurable: true,
});
const localStorage = {};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: false,
isSideBarEnabled: true,
navigationLayout: 'tabs',
});

await mergePersistableValues(localStorage);
Expand All @@ -105,18 +137,22 @@ describe('mergePersistableValues', () => {
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: false,
isSideBarEnabled: true,
}),
});
});

it('should not modify settings when both menubar and sidebar are enabled', async () => {
it('should not modify settings when menubar is already enabled', async () => {
Object.defineProperty(process, 'platform', {
value: 'linux',
writable: true,
configurable: true,
});
const localStorage = {};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: true,
isSideBarEnabled: true,
navigationLayout: 'sidebar',
});

await mergePersistableValues(localStorage);
Expand All @@ -125,7 +161,6 @@ describe('mergePersistableValues', () => {
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: true,
isSideBarEnabled: true,
}),
});
});
Expand All @@ -137,10 +172,11 @@ describe('mergePersistableValues', () => {
autohideMenu: 'true',
};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: false,
isSideBarEnabled: false,
navigationLayout: 'sidebar',
});

await mergePersistableValues(localStorage);
Expand All @@ -149,7 +185,7 @@ describe('mergePersistableValues', () => {
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: false,
isSideBarEnabled: true,
isSideBarEnabled: false,
}),
});
});
Expand All @@ -160,10 +196,11 @@ describe('mergePersistableValues', () => {
'autohideMenu': 'true',
};

mockSelect.mockReturnValueOnce({
mockSelect.mockReturnValue({
...mockInitialValues,
isMenuBarEnabled: false,
isSideBarEnabled: false,
navigationLayout: 'sidebar',
});

await mergePersistableValues(localStorage);
Expand All @@ -172,7 +209,7 @@ describe('mergePersistableValues', () => {
type: APP_SETTINGS_LOADED,
payload: expect.objectContaining({
isMenuBarEnabled: false,
isSideBarEnabled: true,
isSideBarEnabled: false,
}),
});
});
Expand Down
8 changes: 6 additions & 2 deletions src/app/main/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,14 @@ export const mergePersistableValues = async (
},
};

if (!values.isMenuBarEnabled && !values.isSideBarEnabled) {
if (
values.navigationLayout !== 'sidebar' &&
!values.isMenuBarEnabled &&
process.platform === 'linux'
) {
values = {
...values,
isSideBarEnabled: true,
isMenuBarEnabled: true,
};
}

Expand Down
15 changes: 13 additions & 2 deletions src/app/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { createStructuredSelector } from 'reselect';
import { createSelector, createStructuredSelector } from 'reselect';

import type { RootState } from '../store/rootReducer';

export const selectPersistableValues = createStructuredSelector({
// Split into two structured selectors: a single one with 46+ keys exceeds
// TypeScript's type-instantiation depth limit under ts-jest (TS2589).
const selectPersistableValuesA = createStructuredSelector({
currentView: ({ currentView }: RootState) => currentView,
doCheckForUpdatesOnStartup: ({ doCheckForUpdatesOnStartup }: RootState) =>
doCheckForUpdatesOnStartup,
Expand All @@ -13,6 +15,7 @@ export const selectPersistableValues = createStructuredSelector({
isShowWindowOnUnreadChangedEnabled,
}: RootState) => isShowWindowOnUnreadChangedEnabled,
isSideBarEnabled: ({ isSideBarEnabled }: RootState) => isSideBarEnabled,
navigationLayout: ({ navigationLayout }: RootState) => navigationLayout,
isTrayIconEnabled: ({ isTrayIconEnabled }: RootState) => isTrayIconEnabled,
rootWindowState: ({ rootWindowState }: RootState) => rootWindowState,
servers: ({ servers }: RootState) => servers,
Expand Down Expand Up @@ -40,6 +43,9 @@ export const selectPersistableValues = createStructuredSelector({
}: RootState) => isInternalVideoChatWindowEnabled,
isMinimizeOnCloseEnabled: ({ isMinimizeOnCloseEnabled }: RootState) =>
isMinimizeOnCloseEnabled,
});

const selectPersistableValuesB = createStructuredSelector({
isAddNewServersEnabled: ({ isAddNewServersEnabled }: RootState) =>
isAddNewServersEnabled,
isDeveloperModeEnabled: ({ isDeveloperModeEnabled }: RootState) =>
Expand Down Expand Up @@ -92,3 +98,8 @@ export const selectPersistableValues = createStructuredSelector({
}: RootState) => telephonyGlobalShortcutConfig,
isTelephonyEnabled: ({ isTelephonyEnabled }: RootState) => isTelephonyEnabled,
});

export const selectPersistableValues = createSelector(
[selectPersistableValuesA, selectPersistableValuesB],
(a, b) => ({ ...a, ...b })
);
Loading
Loading