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

Fix instances of double translation and guard translation calls using typescript #11443

Merged
merged 12 commits into from
Aug 22, 2023
35 changes: 25 additions & 10 deletions src/@types/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,25 @@ export type Writeable<T> = { -readonly [P in keyof T]: T[P] };

export type ComponentClass = keyof JSX.IntrinsicElements | JSXElementConstructor<any>;

// Utility type for string dot notation for accessing nested object properties
// Based on https://stackoverflow.com/a/58436959
type Join<K, P, S extends string = "."> = K extends string | number
? P extends string | number
? `${K}${"" extends P ? "" : S}${P}`
: never
: never;

type Prev = [never, 0, 1, 2, 3, ...0[]];

/**
* Utility type for string dot notation for accessing nested object properties.
* Based on https://stackoverflow.com/a/58436959
* @example
* {
* "a": {
* "b": {
* "c": "value"
* },
* "d": "foobar"
* }
* }
* will yield a type of `"a.b.c" | "a.b.d"` with Separator="."
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
* @typeParam Target the target type to generate leaf keys for
* @typeParam Separator the separator to use between key segments when accessing nested objects
* @typeParam LeafType the type which leaves of this object extend, used to determine when to stop recursion
* @typeParam MaxDepth the maximum depth to recurse to
* @returns a union type representing all dot (Separator) string notation keys which can access a Leaf (of LeafType)
*/
export type Leaves<Target, Separator extends string = ".", LeafType = string, MaxDepth extends number = 3> = [
MaxDepth,
] extends [never]
Expand All @@ -42,6 +51,12 @@ export type Leaves<Target, Separator extends string = ".", LeafType = string, Ma
: {
[K in keyof Target]-?: Join<K, Leaves<Target[K], Separator, LeafType, Prev[MaxDepth]>, Separator>;
}[keyof Target];
type Prev = [never, 0, 1, 2, 3, ...0[]];
type Join<K, P, S extends string = "."> = K extends string | number
? P extends string | number
? `${K}${"" extends P ? "" : S}${P}`
: never
: never;

export type RecursivePartial<T> = {
[P in keyof T]?: T[P] extends (infer U)[]
Expand Down
8 changes: 4 additions & 4 deletions src/accessibility/KeyboardShortcutUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import { KeyCombo } from "../KeyBindingsManager";
import { IS_MAC, Key } from "../Keyboard";
import { _t, _td, TranslationKey } from "../languageHandler";
import { _t, _td } from "../languageHandler";
import PlatformPeg from "../PlatformPeg";
import SettingsStore from "../settings/SettingsStore";
import {
Expand All @@ -25,9 +25,9 @@ import {
IKeyboardShortcuts,
KeyBindingAction,
KEYBOARD_SHORTCUTS,
KeyboardShortcutSetting,
MAC_ONLY_SHORTCUTS,
} from "./KeyboardShortcuts";
import { IBaseSetting } from "../settings/Settings";

/**
* This function gets the keyboard shortcuts that should be presented in the UI
Expand Down Expand Up @@ -115,7 +115,7 @@ export const getKeyboardShortcuts = (): IKeyboardShortcuts => {
export const getKeyboardShortcutsForUI = (): IKeyboardShortcuts => {
const entries = [...Object.entries(getUIOnlyShortcuts()), ...Object.entries(getKeyboardShortcuts())] as [
KeyBindingAction,
IBaseSetting<KeyCombo>,
KeyboardShortcutSetting,
][];

return entries.reduce((acc, [key, value]) => {
Expand All @@ -130,5 +130,5 @@ export const getKeyboardShortcutValue = (name: KeyBindingAction): KeyCombo | und

export const getKeyboardShortcutDisplayName = (name: KeyBindingAction): string | undefined => {
const keyboardShortcutDisplayName = getKeyboardShortcutsForUI()[name]?.displayName;
return keyboardShortcutDisplayName && _t(keyboardShortcutDisplayName as TranslationKey);
return keyboardShortcutDisplayName && _t(keyboardShortcutDisplayName);
};
4 changes: 3 additions & 1 deletion src/accessibility/KeyboardShortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ export enum KeyBindingAction {
ToggleHiddenEventVisibility = "KeyBinding.toggleHiddenEventVisibility",
}

type KeyboardShortcutSetting = Omit<IBaseSetting<KeyCombo>, "supportedLevels">;
export type KeyboardShortcutSetting = Omit<IBaseSetting<KeyCombo>, "supportedLevels" | "displayName"> & {
displayName?: TranslationKey;
};

// TODO: We should figure out what to do with the keyboard shortcuts that are not handled by KeybindingManager
export type IKeyboardShortcuts = Partial<Record<KeyBindingAction, KeyboardShortcutSetting>>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,11 @@ export default class RolesRoomSettingsTab extends React.Component<IProps> {
return null;
}

const translationKeyForEvent = plEventsToLabels[eventType];
let label: string;
if (plEventsToLabels[eventType]) {
if (translationKeyForEvent) {
const brand = SdkConfig.get("element_call").brand ?? DEFAULTS.element_call.brand;
label = _t(plEventsToLabels[eventType]!, { brand });
label = _t(translationKeyForEvent, { brand });
} else {
label = _t("Send %(eventType)s events", { eventType });
}
Expand Down
20 changes: 16 additions & 4 deletions src/languageHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { logger } from "matrix-js-sdk/src/logger";
import { Optional } from "matrix-events-sdk";
import { MapWithDefault, safeSet } from "matrix-js-sdk/src/utils";

import type EN from "./i18n/strings/en_EN.json";
import type Translations from "./i18n/strings/en_EN.json";
import SettingsStore from "./settings/SettingsStore";
import PlatformPeg from "./PlatformPeg";
import { SettingLevel } from "./settings/SettingLevel";
Expand Down Expand Up @@ -102,12 +102,21 @@ export function getUserLanguage(): string {
}
}

export type TranslationKey = Leaves<typeof EN, "|", string | { other: string }>;
/**
* A type representing the union of possible keys into the translation file using `|` delimiter to access nested fields.
* @example `common|error` to access `error` within the `common` sub-object.
* {
* "common": {
* "error": "Error"
* }
* }
*/
export type TranslationKey = Leaves<typeof Translations, "|", string | { other: string }>;

// Function which only purpose is to mark that a string is translatable
// Does not actually do anything. It's helpful for automatic extraction of translatable strings
export function _td(s: TranslationKey): TranslationKey {
return s as TranslationKey;
return s;
}

/**
Expand Down Expand Up @@ -179,7 +188,10 @@ function safeCounterpartTranslate(text: string, variables?: IVariables): { trans
return translateWithFallback(text, options);
}

export type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode);
/**
* The value a variable or tag can take for a translation interpolation.
*/
type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode);

export interface IVariables {
count?: number;
Expand Down
45 changes: 44 additions & 1 deletion test/TextForEvent-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { EventType, JoinRule, MatrixClient, MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix";
import {
EventType,
HistoryVisibility,
JoinRule,
MatrixClient,
MatrixEvent,
Room,
RoomMember,
} from "matrix-js-sdk/src/matrix";
import { render } from "@testing-library/react";
import { ReactElement } from "react";
import { Mocked, mocked } from "jest-mock";
Expand Down Expand Up @@ -571,4 +579,39 @@ describe("TextForEvent", () => {
).toEqual("@a changed the join rule to a not implemented one");
});
});

describe("textForHistoryVisibilityEvent()", () => {
type TestCase = [string, { result: string }];
const testCases: TestCase[] = [
[
HistoryVisibility.Invited,
{ result: "@a made future room history visible to all room members, from the point they are invited." },
],
[
HistoryVisibility.Joined,
{ result: "@a made future room history visible to all room members, from the point they joined." },
],
[HistoryVisibility.Shared, { result: "@a made future room history visible to all room members." }],
[HistoryVisibility.WorldReadable, { result: "@a made future room history visible to anyone." }],
];

it.each(testCases)(
"returns correct message when room join rule changed to %s",
(historyVisibility, { result }) => {
expect(
textForEvent(
new MatrixEvent({
type: "m.room.history_visibility",
sender: "@a",
content: {
history_visibility: historyVisibility,
},
state_key: "",
}),
mockClient,
),
).toEqual(result);
},
);
});
});
22 changes: 22 additions & 0 deletions test/utils/AutoDiscoveryUtils-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,20 @@ describe("AutoDiscoveryUtils", () => {
expect(logger.error).toHaveBeenCalled();
});

it("throws an error when homeserver config has fail error and recognised error string", () => {
Copy link
Member

Choose a reason for hiding this comment

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

have these AutoDiscovery tests made it into the wrong PR? I'm struggling to see how they are related.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was trying to make the coverage happier, but the bar is too high

const discoveryResult = {
...validIsConfig,
"m.homeserver": {
state: AutoDiscoveryAction.FAIL_ERROR,
error: AutoDiscovery.ERROR_INVALID_HOMESERVER,
},
};
expect(() => AutoDiscoveryUtils.buildValidatedConfigFromDiscovery(serverName, discoveryResult)).toThrow(
"Unexpected error resolving homeserver configuration",
);
expect(logger.error).toHaveBeenCalled();
});

it("throws an error with fallback message identity server config has fail error", () => {
const discoveryResult = {
...validHsConfig,
Expand Down Expand Up @@ -249,4 +263,12 @@ describe("AutoDiscoveryUtils", () => {
);
});
});

describe("authComponentStateForError", () => {
const error = new Error("TEST");

it("should return expected error for the registration page", () => {
expect(AutoDiscoveryUtils.authComponentStateForError(error, "register")).toMatchSnapshot();
});
});
});
26 changes: 26 additions & 0 deletions test/utils/__snapshots__/AutoDiscoveryUtils-test.tsx.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`AutoDiscoveryUtils authComponentStateForError should return expected error for the registration page 1`] = `
{
"serverDeadError": <div>
<strong>
Your Element is misconfigured
</strong>
<div>
<span>
Ask your Element admin to check
<a
href="https://github.com/vector-im/element-web/blob/master/docs/config.md"
rel="noreferrer noopener"
target="_blank"
>
your config
</a>
for incorrect or duplicate entries.
</span>
</div>
</div>,
"serverErrorIsFatal": true,
"serverIsAlive": false,
}
`;
Loading