Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Apply strictNullChecks to src/components/views/spaces/* #10517

Merged
merged 22 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fd1f7ad
Apply to
weeman1337 Apr 18, 2023
8208286
Update translations
weeman1337 Apr 18, 2023
92bbddd
Fix types
weeman1337 Apr 18, 2023
4bba118
Add QuickSettingsButton-test
weeman1337 Apr 18, 2023
bec51d1
Fix QuickSettingsButton-test
weeman1337 Apr 18, 2023
85a0375
Use settings hook
weeman1337 Apr 19, 2023
25eca97
Inline dev tools button
weeman1337 Apr 19, 2023
fee96ec
Roll back theme.ts changes
weeman1337 Apr 19, 2023
3e4c7e0
Now really roll back theme.ts changes
weeman1337 Apr 19, 2023
93740b2
Fix custom themes default
weeman1337 Apr 21, 2023
5d48964
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 May 12, 2023
173c1fc
Fix custom_themes default value
weeman1337 May 12, 2023
5dfcb54
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 May 15, 2023
477ea21
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 May 15, 2023
84d9c1c
Extend theme test
weeman1337 May 16, 2023
51ab2ca
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 May 30, 2023
8adab89
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 May 31, 2023
aed4abe
Merge branch 'develop' into weeman1337/strictify-spaces-views
weeman1337 Jun 28, 2023
f1f6f34
Extend and use ICustomTheme interface
weeman1337 Jun 28, 2023
2ad3177
Update i18n files
weeman1337 Jun 28, 2023
a0def30
Make CustomTheme a type; use more generic type in settings
weeman1337 Jun 28, 2023
ab57131
Merge branch 'develop' into weeman1337/strictify-spaces-views
t3chguy Jun 28, 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
10 changes: 6 additions & 4 deletions src/components/views/spaces/QuickSettingsButton.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2021 The Matrix.org Foundation C.I.C.
Copyright 2021 - 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,7 +33,6 @@ import { Icon as PinUprightIcon } from "../../../../res/img/element-icons/room/p
import { Icon as EllipsisIcon } from "../../../../res/img/element-icons/room/ellipsis.svg";
import { Icon as MembersIcon } from "../../../../res/img/element-icons/room/members.svg";
import { Icon as FavoriteIcon } from "../../../../res/img/element-icons/roomlist/favorite.svg";
import SettingsStore from "../../../settings/SettingsStore";
import Modal from "../../../Modal";
import DevtoolsDialog from "../dialogs/DevtoolsDialog";
import { SdkContextClass } from "../../../contexts/SDKContext";
Expand All @@ -46,6 +45,9 @@ const QuickSettingsButton: React.FC<{
const { [MetaSpace.Favourites]: favouritesEnabled, [MetaSpace.People]: peopleEnabled } =
useSettingValue<Record<MetaSpace, boolean>>("Spaces.enabledMetaSpaces");

const currentRoomId = SdkContextClass.instance.roomViewStore.getRoomId();
const developerModeEnabled = useSettingValue("developerMode");

let contextMenu: JSX.Element | undefined;
if (menuDisplayed && handle.current) {
contextMenu = (
Expand All @@ -68,14 +70,14 @@ const QuickSettingsButton: React.FC<{
{_t("All settings")}
</AccessibleButton>

{SettingsStore.getValue("developerMode") && SdkContextClass.instance.roomViewStore.getRoomId() && (
{currentRoomId && developerModeEnabled && (
<AccessibleButton
onClick={() => {
closeMenu();
Modal.createDialog(
DevtoolsDialog,
{
roomId: SdkContextClass.instance.roomViewStore.getRoomId()!,
roomId: currentRoomId,
},
"mx_DevtoolsDialog_wrapper",
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/spaces/SpacePublicShare.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2021 The Matrix.org Foundation C.I.C.
Copyright 2021 - 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
11 changes: 6 additions & 5 deletions src/components/views/spaces/SpaceTreeLevel.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2021 The Matrix.org Foundation C.I.C.
Copyright 2021 - 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -95,9 +95,9 @@ export const SpaceButton = forwardRef<HTMLElement, IButtonProps>(
}

let notifBadge;
if (notificationState) {
if (space && notificationState) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this interests you, but there's now a Typeguards.ts file that has an isNotUndefined helper typeguard for situations like this.

let ariaLabel = _t("Jump to first unread room.");
if (space?.getMyMembership() === "invite") {
if (space.getMyMembership() === "invite") {
ariaLabel = _t("Jump to first invite.");
}

Expand Down Expand Up @@ -133,8 +133,9 @@ export const SpaceButton = forwardRef<HTMLElement, IButtonProps>(
}

const viewSpaceHome = (): void =>
defaultDispatcher.dispatch({ action: Action.ViewRoom, room_id: space.roomId });
const activateSpace = (): void => SpaceStore.instance.setActiveSpace(spaceKey ?? space.roomId);
// space is set here because of the assignment condition of onClick
defaultDispatcher.dispatch({ action: Action.ViewRoom, room_id: space!.roomId });
const activateSpace = (): void => SpaceStore.instance.setActiveSpace(spaceKey ?? space?.roomId ?? "");
const onClick = props.onClick ?? (selected && space ? viewSpaceHome : activateSpace);

return (
Expand Down
2 changes: 1 addition & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1261,9 +1261,9 @@
"Ignore (%(counter)s)": "Ignore (%(counter)s)",
"Verify Session": "Verify Session",
"Accept <policyLink /> to continue:": "Accept <policyLink /> to continue:",
"Developer tools": "Developer tools",
"Quick settings": "Quick settings",
"All settings": "All settings",
"Developer tools": "Developer tools",
"Pin to sidebar": "Pin to sidebar",
"More options": "More options",
"Settings": "Settings",
Expand Down
13 changes: 11 additions & 2 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import { FontWatcher } from "./watchers/FontWatcher";
import RustCryptoSdkController from "./controllers/RustCryptoSdkController";
import ServerSupportUnstableFeatureController from "./controllers/ServerSupportUnstableFeatureController";
import { WatchManager } from "./WatchManager";
import { CustomTheme } from "../theme";

export const defaultWatchManager = new WatchManager();

Expand Down Expand Up @@ -111,7 +112,15 @@ export const labGroupNames: Record<LabGroup, string> = {
[LabGroup.Developer]: _td("Developer"),
};

export type SettingValueType = boolean | number | string | number[] | string[] | Record<string, unknown> | null;
export type SettingValueType =
| boolean
| number
| string
| number[]
| string[]
| Record<string, unknown>
| CustomTheme[]
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
| null;

export interface IBaseSetting<T extends SettingValueType = SettingValueType> {
isFeature?: false | undefined;
Expand Down Expand Up @@ -653,7 +662,7 @@ export const SETTINGS: { [setting: string]: ISetting } = {
},
"custom_themes": {
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
default: [],
default: [] as CustomTheme[],
},
"use_system_theme": {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
Expand Down
23 changes: 17 additions & 6 deletions src/theme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ limitations under the License.
*/

import { compare } from "matrix-js-sdk/src/utils";
import { logger } from "matrix-js-sdk/src/logger";

import { _t } from "./languageHandler";
import SettingsStore from "./settings/SettingsStore";
Expand All @@ -34,7 +35,8 @@ interface IFontFaces extends Omit<Record<(typeof allowedFontFaceProps)[number],
}[];
}

interface ICustomTheme {
export interface CustomTheme {
name: string;
colors: {
[key: string]: string;
};
Expand Down Expand Up @@ -79,11 +81,20 @@ export function enumerateThemes(): { [key: string]: string } {
"light-high-contrast": _t("Light high contrast"),
"dark": _t("Dark"),
};
const customThemes = SettingsStore.getValue("custom_themes");
const customThemes = SettingsStore.getValue("custom_themes") || [];
const customThemeNames: Record<string, string> = {};
for (const { name } of customThemes) {
customThemeNames[`custom-${name}`] = name;

try {
for (const { name } of customThemes) {
customThemeNames[`custom-${name}`] = name;
}
} catch (err) {
logger.warn("Error loading custom themes", {
err,
customThemes,
});
}

return Object.assign({}, customThemeNames, BUILTIN_THEMES);
}

Expand Down Expand Up @@ -166,7 +177,7 @@ function generateCustomFontFaceCSS(faces: IFontFaces[]): string {
.join("\n");
}

function setCustomThemeVars(customTheme: ICustomTheme): void {
function setCustomThemeVars(customTheme: CustomTheme): void {
const { style } = document.body;

function setCSSColorVariable(name: string, hexColor: string, doPct = true): void {
Expand Down Expand Up @@ -209,7 +220,7 @@ function setCustomThemeVars(customTheme: ICustomTheme): void {
}
}

export function getCustomTheme(themeName: string): ICustomTheme {
export function getCustomTheme(themeName: string): CustomTheme {
// set css variables
const customThemes = SettingsStore.getValue("custom_themes");
if (!customThemes) {
Expand Down
96 changes: 96 additions & 0 deletions test/components/views/spaces/QuickSettingsButton-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";
import { render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { mocked } from "jest-mock";

import QuickSettingsButton from "../../../../src/components/views/spaces/QuickSettingsButton";
import SettingsStore from "../../../../src/settings/SettingsStore";
import { SdkContextClass } from "../../../../src/contexts/SDKContext";

describe("QuickSettingsButton", () => {
const roomId = "!room:example.com";

const renderQuickSettingsButton = () => {
render(<QuickSettingsButton isPanelCollapsed={true} />);
};

const getQuickSettingsButton = () => {
return screen.getByRole("button", { name: "Quick settings" });
};

const openQuickSettings = async () => {
await userEvent.click(getQuickSettingsButton());
await screen.findByText("Quick settings");
};

it("should render the quick settings button", () => {
renderQuickSettingsButton();
expect(getQuickSettingsButton()).toBeInTheDocument();
});

describe("when the quick settings are open", () => {
beforeEach(async () => {
renderQuickSettingsButton();
await openQuickSettings();
});

it("should not render the »Developer tools« button", () => {
renderQuickSettingsButton();
expect(screen.queryByText("Developer tools")).not.toBeInTheDocument();
});
});

describe("when developer mode is enabled", () => {
beforeEach(() => {
jest.spyOn(SettingsStore, "getValue").mockImplementation((setting) => setting === "developerMode");
renderQuickSettingsButton();
});

afterEach(() => {
mocked(SettingsStore.getValue).mockRestore();
});

describe("and no room is viewed", () => {
it("should not render the »Developer tools« button", () => {
renderQuickSettingsButton();
expect(screen.queryByText("Developer tools")).not.toBeInTheDocument();
});
});

describe("and a room is viewed", () => {
beforeEach(() => {
jest.spyOn(SdkContextClass.instance.roomViewStore, "getRoomId").mockReturnValue(roomId);
});

afterEach(() => {
mocked(SdkContextClass.instance.roomViewStore.getRoomId).mockRestore();
});

describe("and the quick settings are open", () => {
beforeEach(async () => {
await openQuickSettings();
});

it("should render the »Developer tools« button", () => {
expect(screen.getByRole("button", { name: "Developer tools" })).toBeInTheDocument();
});
});
});
});
});
24 changes: 23 additions & 1 deletion test/theme-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { setTheme } from "../src/theme";
import SettingsStore from "../src/settings/SettingsStore";
import { enumerateThemes, setTheme } from "../src/theme";

describe("theme", () => {
describe("setTheme", () => {
Expand Down Expand Up @@ -124,4 +125,25 @@ describe("theme", () => {
});
});
});

describe("enumerateThemes", () => {
it("should return a list of themes", () => {
jest.spyOn(SettingsStore, "getValue").mockReturnValue([{ name: "pink" }]);
expect(enumerateThemes()).toEqual({
"light": "Light",
"light-high-contrast": "Light high contrast",
"dark": "Dark",
"custom-pink": "pink",
});
});

it("should be robust to malformed custom_themes values", () => {
jest.spyOn(SettingsStore, "getValue").mockReturnValue([23]);
expect(enumerateThemes()).toEqual({
"light": "Light",
"light-high-contrast": "Light high contrast",
"dark": "Dark",
});
});
});
});