From dd6fc124d784c76aab8b9a3d236895cedf4e7a5f Mon Sep 17 00:00:00 2001 From: Kerry <kerrya@element.io> Date: Tue, 28 Feb 2023 21:55:59 +1300 Subject: [PATCH] Apply strictNullChecks to src/components/views/location/* (#10249 * strict fixes * accessiblebutton without onClick explicit * strict fix for UserMenu BaseAvatar --- src/components/structures/UserMenu.tsx | 2 +- src/components/views/avatars/BaseAvatar.tsx | 2 +- src/components/views/location/LocationButton.tsx | 6 +++--- src/components/views/location/LocationPicker.tsx | 12 ++++++------ .../views/location/LocationShareMenu.tsx | 7 ++++--- .../views/location/LocationViewDialog.tsx | 4 ++-- src/components/views/location/Map.tsx | 3 +++ src/components/views/location/ShareType.tsx | 16 ++++++++++------ src/components/views/location/SmartMarker.tsx | 12 ++++++++---- src/components/views/location/shareLocation.ts | 7 ++++--- .../views/location/LocationShareMenu-test.tsx | 5 +++-- test/components/views/spaces/SpacePanel-test.tsx | 1 + 12 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/components/structures/UserMenu.tsx b/src/components/structures/UserMenu.tsx index aea40bbe57..4f52b2c970 100644 --- a/src/components/structures/UserMenu.tsx +++ b/src/components/structures/UserMenu.tsx @@ -432,7 +432,7 @@ export default class UserMenu extends React.Component<IProps, IState> { public render(): React.ReactNode { const avatarSize = 32; // should match border-radius of the avatar - const userId = MatrixClientPeg.get().getUserId(); + const userId = MatrixClientPeg.get().getSafeUserId(); const displayName = OwnProfileStore.instance.displayName || userId; const avatarUrl = OwnProfileStore.instance.getHttpAvatarUrl(avatarSize); diff --git a/src/components/views/avatars/BaseAvatar.tsx b/src/components/views/avatars/BaseAvatar.tsx index 469c5fbb2a..59553e3d7f 100644 --- a/src/components/views/avatars/BaseAvatar.tsx +++ b/src/components/views/avatars/BaseAvatar.tsx @@ -32,7 +32,7 @@ import { toPx } from "../../../utils/units"; import { _t } from "../../../languageHandler"; interface IProps { - name: string; // The name (first initial used as default) + name?: string; // The name (first initial used as default) idName?: string; // ID for generating hash colours title?: string; // onHover title text url?: string; // highest priority of them all, shortcut to set in urls[0] diff --git a/src/components/views/location/LocationButton.tsx b/src/components/views/location/LocationButton.tsx index 056caaa3da..08e13537d2 100644 --- a/src/components/views/location/LocationButton.tsx +++ b/src/components/views/location/LocationButton.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { ReactElement, SyntheticEvent, useContext } from "react"; +import React, { ReactNode, SyntheticEvent, useContext } from "react"; import classNames from "classnames"; import { RoomMember } from "matrix-js-sdk/src/models/room-member"; import { IEventRelation } from "matrix-js-sdk/src/models/event"; @@ -41,9 +41,9 @@ export const LocationButton: React.FC<IProps> = ({ roomId, sender, menuPosition, overflowMenuCloser?.(); }; - let contextMenu: ReactElement; + let contextMenu: ReactNode = null; if (menuDisplayed) { - const position = menuPosition ?? aboveLeftOf(button.current.getBoundingClientRect()); + const position = menuPosition ?? (button.current && aboveLeftOf(button.current.getBoundingClientRect())) ?? {}; contextMenu = ( <LocationShareMenu diff --git a/src/components/views/location/LocationPicker.tsx b/src/components/views/location/LocationPicker.tsx index 503d4d8688..c012a1ab78 100644 --- a/src/components/views/location/LocationPicker.tsx +++ b/src/components/views/location/LocationPicker.tsx @@ -52,9 +52,9 @@ const isSharingOwnLocation = (shareType: LocationShareType): boolean => class LocationPicker extends React.Component<ILocationPickerProps, IState> { public static contextType = MatrixClientContext; public context!: React.ContextType<typeof MatrixClientContext>; - private map?: maplibregl.Map = null; - private geolocate?: maplibregl.GeolocateControl = null; - private marker?: maplibregl.Marker = null; + private map?: maplibregl.Map; + private geolocate?: maplibregl.GeolocateControl; + private marker?: maplibregl.Marker; public constructor(props: ILocationPickerProps) { super(props); @@ -100,7 +100,7 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> { }); this.map.on("load", () => { - this.geolocate.trigger(); + this.geolocate?.trigger(); }); this.geolocate.on("error", this.onGeolocateError); @@ -120,7 +120,7 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> { } catch (e) { logger.error("Failed to render map", e); const errorType = - e?.message === LocationShareError.MapStyleUrlNotConfigured + (e as Error)?.message === LocationShareError.MapStyleUrlNotConfigured ? LocationShareError.MapStyleUrlNotConfigured : LocationShareError.Default; this.setState({ error: errorType }); @@ -141,7 +141,7 @@ class LocationPicker extends React.Component<ILocationPickerProps, IState> { offset: [0, -1], }) .setLngLat(new maplibregl.LngLat(0, 0)) - .addTo(this.map); + .addTo(this.map!); }; private updateStyleUrl = (clientWellKnown: IClientWellKnown): void => { diff --git a/src/components/views/location/LocationShareMenu.tsx b/src/components/views/location/LocationShareMenu.tsx index 5968d50c47..3fc369e3d5 100644 --- a/src/components/views/location/LocationShareMenu.tsx +++ b/src/components/views/location/LocationShareMenu.tsx @@ -64,14 +64,15 @@ const LocationShareMenu: React.FC<Props> = ({ menuPosition, onFinished, sender, ); const displayName = OwnProfileStore.instance.displayName; + const userId = matrixClient.getSafeUserId(); const onLocationSubmit = shareType === LocationShareType.Live - ? shareLiveLocation(matrixClient, roomId, displayName, openMenu) - : shareLocation(matrixClient, roomId, shareType, relation, openMenu); + ? shareLiveLocation(matrixClient, roomId, displayName || userId, openMenu) + : shareLocation(matrixClient, roomId, shareType ?? LocationShareType.Own, relation, openMenu); const onLiveShareEnableSubmit = (): void => { - SettingsStore.setValue("feature_location_share_live", undefined, SettingLevel.DEVICE, true); + SettingsStore.setValue("feature_location_share_live", null, SettingLevel.DEVICE, true); }; const shouldAdvertiseLiveLabsFlag = shareType === LocationShareType.Live && !isLiveShareEnabled; diff --git a/src/components/views/location/LocationViewDialog.tsx b/src/components/views/location/LocationViewDialog.tsx index a84ec29c3b..042e37a111 100644 --- a/src/components/views/location/LocationViewDialog.tsx +++ b/src/components/views/location/LocationViewDialog.tsx @@ -31,7 +31,7 @@ interface IProps extends IDialogProps { } interface IState { - error: Error; + error?: Error; } /** @@ -58,7 +58,7 @@ export default class LocationViewDialog extends React.Component<IProps, IState> const { mxEvent } = this.props; // only pass member to marker when should render avatar marker - const markerRoomMember = isSelfLocation(mxEvent.getContent()) ? mxEvent.sender : undefined; + const markerRoomMember = (isSelfLocation(mxEvent.getContent()) && mxEvent.sender) || undefined; const geoUri = locationEventGeoUri(mxEvent); return ( <BaseDialog className="mx_LocationViewDialog" onFinished={this.props.onFinished} fixedWidth={false}> diff --git a/src/components/views/location/Map.tsx b/src/components/views/location/Map.tsx index 22a4bda63e..2fc9f97280 100644 --- a/src/components/views/location/Map.tsx +++ b/src/components/views/location/Map.tsx @@ -70,6 +70,9 @@ const useMapWithStyle = ({ if (map && centerGeoUri) { try { const coords = parseGeoUri(centerGeoUri); + if (!coords) { + throw new Error("Invalid geo URI"); + } map.setCenter({ lon: coords.longitude, lat: coords.latitude }); } catch (_error) { logger.error("Could not set map center"); diff --git a/src/components/views/location/ShareType.tsx b/src/components/views/location/ShareType.tsx index a794b36e83..81d232e6f5 100644 --- a/src/components/views/location/ShareType.tsx +++ b/src/components/views/location/ShareType.tsx @@ -20,7 +20,7 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext"; import { _t } from "../../../languageHandler"; import { OwnProfileStore } from "../../../stores/OwnProfileStore"; import BaseAvatar from "../avatars/BaseAvatar"; -import AccessibleButton from "../elements/AccessibleButton"; +import AccessibleButton, { ButtonEvent } from "../elements/AccessibleButton"; import Heading from "../typography/Heading"; import { Icon as LocationIcon } from "../../../../res/img/element-icons/location.svg"; import { LocationShareType } from "./shareLocation"; @@ -28,11 +28,11 @@ import StyledLiveBeaconIcon from "../beacon/StyledLiveBeaconIcon"; const UserAvatar: React.FC = () => { const matrixClient = useContext(MatrixClientContext); - const userId = matrixClient.getUserId(); - const displayName = OwnProfileStore.instance.displayName; + const userId = matrixClient.getSafeUserId(); + const displayName = OwnProfileStore.instance.displayName ?? undefined; // 40 - 2px border const avatarSize = 36; - const avatarUrl = OwnProfileStore.instance.getHttpAvatarUrl(avatarSize); + const avatarUrl = OwnProfileStore.instance.getHttpAvatarUrl(avatarSize) ?? undefined; return ( <div className={`mx_ShareType_option-icon ${LocationShareType.Own}`}> @@ -49,9 +49,13 @@ const UserAvatar: React.FC = () => { ); }; -type ShareTypeOptionProps = HTMLAttributes<Element> & { label: string; shareType: LocationShareType }; +type ShareTypeOptionProps = HTMLAttributes<Element> & { + label: string; + shareType: LocationShareType; + onClick?: ((e: ButtonEvent) => void | Promise<void>) | null; +}; const ShareTypeOption: React.FC<ShareTypeOptionProps> = ({ onClick, label, shareType, ...rest }) => ( - <AccessibleButton element="button" className="mx_ShareType_option" onClick={onClick} {...rest}> + <AccessibleButton element="button" className="mx_ShareType_option" onClick={onClick ?? null} {...rest}> {shareType === LocationShareType.Own && <UserAvatar />} {shareType === LocationShareType.Pin && ( <LocationIcon className={`mx_ShareType_option-icon ${LocationShareType.Pin}`} /> diff --git a/src/components/views/location/SmartMarker.tsx b/src/components/views/location/SmartMarker.tsx index 225cfbdcdc..f082bce8d8 100644 --- a/src/components/views/location/SmartMarker.tsx +++ b/src/components/views/location/SmartMarker.tsx @@ -33,9 +33,11 @@ const useMapMarker = ( return; } const coords = parseGeoUri(geoUri); - const newMarker = createMarker(coords, element); - newMarker.addTo(map); - setMarker(newMarker); + if (coords) { + const newMarker = createMarker(coords, element); + newMarker.addTo(map); + setMarker(newMarker); + } }, [marker, geoUri, map], ); @@ -43,7 +45,9 @@ const useMapMarker = ( useEffect(() => { if (marker) { const coords = parseGeoUri(geoUri); - marker.setLngLat({ lon: coords.longitude, lat: coords.latitude }); + if (coords) { + marker.setLngLat({ lon: coords.longitude, lat: coords.latitude }); + } } }, [marker, geoUri]); diff --git a/src/components/views/location/shareLocation.ts b/src/components/views/location/shareLocation.ts index 19234cde31..808ef6e4ca 100644 --- a/src/components/views/location/shareLocation.ts +++ b/src/components/views/location/shareLocation.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { MatrixClient } from "matrix-js-sdk/src/client"; +import { IContent } from "matrix-js-sdk/src/matrix"; import { MatrixError } from "matrix-js-sdk/src/http-api"; import { makeLocationContent, makeBeaconInfoContent } from "matrix-js-sdk/src/content-helpers"; import { logger } from "matrix-js-sdk/src/logger"; @@ -94,7 +95,7 @@ const getDefaultErrorParams = ( return { modalParams, errorMessage }; }; -const handleShareError = (error: Error, openMenu: () => void, shareType: LocationShareType): void => { +const handleShareError = (error: unknown, openMenu: () => void, shareType: LocationShareType): void => { const { modalParams, errorMessage } = (error as MatrixError).errcode === "M_FORBIDDEN" ? getPermissionsErrorParams(shareType) @@ -135,9 +136,9 @@ export const shareLocation = async ({ uri, timestamp }): Promise<void> => { if (!uri) return; try { - const threadId = relation?.rel_type === THREAD_RELATION_TYPE.name ? relation.event_id : null; + const threadId = (relation?.rel_type === THREAD_RELATION_TYPE.name && relation?.event_id) || null; const assetType = shareType === LocationShareType.Pin ? LocationAssetType.Pin : LocationAssetType.Self; - const content = makeLocationContent(undefined, uri, timestamp, undefined, assetType); + const content = makeLocationContent(undefined, uri, timestamp, undefined, assetType) as IContent; await doMaybeLocalRoomAction( roomId, (actualRoomId: string) => client.sendMessage(actualRoomId, threadId, content), diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 42331429d3..9ee667f319 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -33,6 +33,7 @@ import { LocationShareType } from "../../../../src/components/views/location/sha import { flushPromisesWithFakeTimers, getMockClientWithEventEmitter, + mockClientMethodsUser, setupAsyncStoreWithClient, } from "../../../test-utils"; import Modal from "../../../../src/Modal"; @@ -74,7 +75,7 @@ jest.mock("../../../../src/Modal", () => ({ describe("<LocationShareMenu />", () => { const userId = "@ernie:server.org"; const mockClient = getMockClientWithEventEmitter({ - getUserId: jest.fn().mockReturnValue(userId), + ...mockClientMethodsUser(userId), getClientWellKnown: jest.fn().mockResolvedValue({ map_style_url: "maps.com", }), @@ -334,7 +335,7 @@ describe("<LocationShareMenu />", () => { expect(SettingsStore.setValue).toHaveBeenCalledWith( "feature_location_share_live", - undefined, + null, SettingLevel.DEVICE, true, ); diff --git a/test/components/views/spaces/SpacePanel-test.tsx b/test/components/views/spaces/SpacePanel-test.tsx index 241b1ded73..8ed108ee39 100644 --- a/test/components/views/spaces/SpacePanel-test.tsx +++ b/test/components/views/spaces/SpacePanel-test.tsx @@ -48,6 +48,7 @@ jest.mock("../../../../src/customisations/helpers/UIComponents", () => ({ describe("<SpacePanel />", () => { const mockClient = { getUserId: jest.fn().mockReturnValue("@test:test"), + getSafeUserId: jest.fn().mockReturnValue("@test:test"), isGuest: jest.fn(), getAccountData: jest.fn(), } as unknown as MatrixClient;