nikrozman/element-web
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Address code smells from #30453 (#31083)

Browse Source 
* check recents length

* Improve sorting logic in capabilities dialog

* Fix allowedHosts regex array check

* Remove double negative in filter checks

* prettier
This commit is contained in:
David Langley
2025-10-29 08:52:17 +00:00
committed by GitHub
parent 3e809cd661
commit 80a7de4314
7 changed files with 207 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/components/views/dialogs/InviteDialog.tsx
View File

@@ -415,7 +415,7 @@ export default class InviteDialog extends React.PureComponent<Props, IInviteDial
// We mutate the given set so that any later callers avoid duplicating these users
excludedTargetIds.add(userId);
}
if (!recents) logger.warn("[Invite:Recents] No recents to suggest!");
if (recents.length === 0) logger.warn("[Invite:Recents] No recents to suggest!");
// Sort the recents by last active to save us time later
recents.sort((a, b) => b.lastActive - a.lastActive);

11
src/components/views/dialogs/WidgetCapabilitiesPromptDialog.tsx
View File

@@ -91,12 +91,13 @@ export default class WidgetCapabilitiesPromptDialog extends React.PureComponent<
const isTimelineA = isTimelineCapability(capA);
const isTimelineB = isTimelineCapability(capB);
if (!isTimelineA && !isTimelineB) return lexicographicCompare(capA, capB);
if (isTimelineA && !isTimelineB) return 1;
if (!isTimelineA && isTimelineB) return -1;
if (isTimelineA && isTimelineB) return lexicographicCompare(capA, capB);
// Sort timeline capabilities to the bottom, non-timeline to the top
if (isTimelineA !== isTimelineB) {
return isTimelineA ? 1 : -1;
}
return 0;
// Both are the same type (both timeline or both non-timeline), sort lexicographically
return lexicographicCompare(capA, capB);
});
const checkboxRows = orderedCapabilities.map(([cap, isChecked], i) => {
const text = CapabilityText.for(cap, this.props.widgetKind);

8
src/components/views/dialogs/spotlight/SpotlightDialog.tsx
View File

@@ -1144,7 +1144,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
if (rovingContext.state.activeNode && rovingContext.state.nodes.length > 0) {
let nodes = rovingContext.state.nodes;
if (!query && !filter !== null) {
if (!query && filter === null) {
// If the current selection is not in the recently viewed row then only include the
// first recently viewed so that is the target when the user is switching into recently viewed.
const keptRecentlyViewedRef = nodeIsForRecentlyViewed(rovingContext.state.activeNode)
@@ -1164,7 +1164,7 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
// only handle these keys when we are in the recently viewed row of options
if (
!query &&
!filter !== null &&
filter === null &&
rovingContext.state.activeNode &&
rovingContext.state.nodes.length > 0 &&
nodeIsForRecentlyViewed(rovingContext.state.activeNode)
@@ -1226,8 +1226,8 @@ const SpotlightDialog: React.FC<IProps> = ({ initialText = "", initialFilter = n
<>
<kbd>↓</kbd>
<kbd>↑</kbd>
{!filter !== null && !query && <kbd>←</kbd>}
{!filter !== null && !query && <kbd>→</kbd>}
{filter === null && !query && <kbd>←</kbd>}
{filter === null && !query && <kbd>→</kbd>}
</>
),
},

2
src/utils/permalinks/Permalinks.ts
View File

@@ -217,7 +217,7 @@ export class RoomPermalinkCreator {
const allowed = aclEvent.getContent<{ allow: string[] }>().allow;
allowedHostsRegexps = []; // we don't want to use the default rule here
if (Array.isArray(denied)) {
if (Array.isArray(allowed)) {
allowed.forEach((h) => allowedHostsRegexps.push(getRegex(h)));
}
}

35
test/unit-tests/components/views/dialogs/SpotlightDialog-test.tsx
View File

@@ -664,4 +664,39 @@ describe("Spotlight Dialog", () => {
}),
);
});
describe("keyboard prompt filter and query checks", () => {
it("should show left and right arrow keys in keyboard hint when filter is null and no query", async () => {
render(<SpotlightDialog onFinished={() => null} />);
jest.advanceTimersByTime(200);
await flushPromisesWithFakeTimers();
const keyboardPrompt = document.querySelector("#mx_SpotlightDialog_keyboardPrompt");
expect(keyboardPrompt).toBeInTheDocument();
expect(keyboardPrompt?.textContent).toContain("←");
expect(keyboardPrompt?.textContent).toContain("→");
});
it("should not show left and right arrow keys in keyboard hint when filter is set", async () => {
render(<SpotlightDialog initialFilter={Filter.People} onFinished={() => null} />);
jest.advanceTimersByTime(200);
await flushPromisesWithFakeTimers();
const keyboardPrompt = document.querySelector("#mx_SpotlightDialog_keyboardPrompt");
expect(keyboardPrompt).toBeInTheDocument();
expect(keyboardPrompt?.textContent).not.toContain("←");
expect(keyboardPrompt?.textContent).not.toContain("→");
});
it("should not show left and right arrow keys in keyboard hint when query is present", async () => {
render(<SpotlightDialog initialText="test query" onFinished={() => null} />);
jest.advanceTimersByTime(200);
await flushPromisesWithFakeTimers();
const keyboardPrompt = document.querySelector("#mx_SpotlightDialog_keyboardPrompt");
expect(keyboardPrompt).toBeInTheDocument();
expect(keyboardPrompt?.textContent).not.toContain("←");
expect(keyboardPrompt?.textContent).not.toContain("→");
});
});
});

133
test/unit-tests/components/views/dialogs/WidgetCapabilitiesPromptDialog-test.tsx Normal file
View File

@@ -0,0 +1,133 @@
/*
Copyright 2025 New Vector Ltd.
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
Please see LICENSE files in the repository root for full details.
*/
import React from "react";
import { render, screen } from "jest-matrix-react";
import { WidgetKind } from "matrix-widget-api";
import WidgetCapabilitiesPromptDialog from "../../../../../src/components/views/dialogs/WidgetCapabilitiesPromptDialog";
import { stubClient } from "../../../../test-utils";
describe("WidgetCapabilitiesPromptDialog", () => {
const mockWidget = {
id: "test-widget",
name: "Test Widget",
} as any;
const onFinished = jest.fn();
beforeEach(() => {
stubClient();
onFinished.mockClear();
});
describe("capability sorting", () => {
it("should sort non-timeline capabilities before timeline capabilities", () => {
// Create a mix of timeline and non-timeline capabilities
const capabilities = new Set([
"org.matrix.msc2762.timeline:*",
"org.matrix.msc2762.receive.event:m.room.message",
"org.matrix.msc2762.timeline:!room:example.com",
"org.matrix.msc2931.navigate",
"org.matrix.msc2762.receive.state_event:m.room.member#@user:example.com",
]);
const { container } = render(
<WidgetCapabilitiesPromptDialog
requestedCapabilities={capabilities}
widget={mockWidget}
widgetKind={WidgetKind.Room}
onFinished={onFinished}
/>,
);
const checkboxes = container.querySelectorAll(".mx_WidgetCapabilitiesPromptDialog_cap");
// Verify that we have all the capabilities rendered
expect(checkboxes.length).toBe(5);
// The dialog should render successfully with the mixed capabilities
expect(screen.getByText("Approve widget permissions")).toBeInTheDocument();
});
it("should sort capabilities lexicographically within the same type", () => {
// Create multiple non-timeline capabilities
const capabilities = new Set([
"org.matrix.msc2931.navigate",
"org.matrix.msc2762.receive.event:m.room.message",
"org.matrix.msc2762.receive.state_event:m.room.member",
]);
render(
<WidgetCapabilitiesPromptDialog
requestedCapabilities={capabilities}
widget={mockWidget}
widgetKind={WidgetKind.Room}
onFinished={onFinished}
/>,
);
// The dialog should render without errors and show the capabilities
expect(screen.getByText("Approve widget permissions")).toBeInTheDocument();
});
it("should handle only timeline capabilities", () => {
const capabilities = new Set([
"org.matrix.msc2762.timeline:!room1:example.com",
"org.matrix.msc2762.timeline:!room2:example.com",
"org.matrix.msc2762.timeline:*",
]);
const { container } = render(
<WidgetCapabilitiesPromptDialog
requestedCapabilities={capabilities}
widget={mockWidget}
widgetKind={WidgetKind.Room}
onFinished={onFinished}
/>,
);
const checkboxes = container.querySelectorAll(".mx_WidgetCapabilitiesPromptDialog_cap");
expect(checkboxes.length).toBe(3);
});
it("should handle only non-timeline capabilities", () => {
const capabilities = new Set([
"org.matrix.msc2931.navigate",
"org.matrix.msc2762.receive.event:m.room.message",
]);
const { container } = render(
<WidgetCapabilitiesPromptDialog
requestedCapabilities={capabilities}
widget={mockWidget}
widgetKind={WidgetKind.Room}
onFinished={onFinished}
/>,
);
const checkboxes = container.querySelectorAll(".mx_WidgetCapabilitiesPromptDialog_cap");
expect(checkboxes.length).toBe(2);
});
it("should handle empty capabilities", () => {
const capabilities = new Set<string>([]);
const { container } = render(
<WidgetCapabilitiesPromptDialog
requestedCapabilities={capabilities}
widget={mockWidget}
widgetKind={WidgetKind.Room}
onFinished={onFinished}
/>,
);
const checkboxes = container.querySelectorAll(".mx_WidgetCapabilitiesPromptDialog_cap");
expect(checkboxes.length).toBe(0);
});
});
});

27
test/unit-tests/utils/permalinks/Permalinks-test.ts
View File

@@ -334,6 +334,33 @@ describe("Permalinks", function () {
expect(creator.serverCandidates![0]).toEqual("evilcorp.com");
});
it("should handle when ACL allow is not an array", function () {
const roomId = "!fake:example.org";
const room = mockRoom(roomId, [makeMemberWithPL(roomId, "@alice:goodcorp.com", 100)], {
deny: ["*.evilcorp.com"],
allow: "not-an-array" as any, // Test malformed data
});
const creator = new RoomPermalinkCreator(room);
creator.load();
// Should fall back to default behavior (no allowed servers list = allow none)
expect(creator.serverCandidates).toBeTruthy();
expect(creator.serverCandidates!.length).toBe(0);
});
it("should handle when ACL deny is not an array", function () {
const roomId = "!fake:example.org";
const room = mockRoom(roomId, [makeMemberWithPL(roomId, "@alice:goodcorp.com", 100)], {
deny: "not-an-array" as any, // Test malformed data
allow: ["*"],
});
const creator = new RoomPermalinkCreator(room);
creator.load();
// Should not crash and still allow the server
expect(creator.serverCandidates).toBeTruthy();
expect(creator.serverCandidates!.length).toBe(1);
expect(creator.serverCandidates![0]).toEqual("goodcorp.com");
});
it("should generate an event permalink for room IDs with no candidate servers", function () {
const room = mockRoom("!somewhere:example.org", []);
const creator = new RoomPermalinkCreator(room);