From 296ed2273e0b0bbb61bf0fadf617e939062fbb88 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 15 May 2023 14:33:49 +0100 Subject: [PATCH] Use semantic list elements for menu lists and tab lists (#10902) * Use semantic list elements for pop up menu lists * Use semantic list elements for tab lists * Fix tests * Update snapshot --- .../context_menus/_IconizedContextMenu.pcss | 3 ++ src/components/structures/TabbedView.tsx | 5 +- .../context_menus/IconizedContextMenu.tsx | 5 +- .../__snapshots__/TabbedView-test.tsx.snap | 16 +++--- .../context_menus/MessageContextMenu-test.tsx | 50 +++++++++---------- .../RoomGeneralContextMenu-test.tsx.snap | 18 ++++--- .../SpaceContextMenu-test.tsx.snap | 21 ++++---- .../RoomSettingsDialog-test.tsx.snap | 20 ++++---- .../UserSettingsDialog-test.tsx.snap | 36 ++++++------- 9 files changed, 92 insertions(+), 82 deletions(-) diff --git a/res/css/views/context_menus/_IconizedContextMenu.pcss b/res/css/views/context_menus/_IconizedContextMenu.pcss index 3a1bb0eba2..581d0ca5e2 100644 --- a/res/css/views/context_menus/_IconizedContextMenu.pcss +++ b/res/css/views/context_menus/_IconizedContextMenu.pcss @@ -18,6 +18,9 @@ limitations under the License. .mx_IconizedContextMenu { min-width: 146px; width: max-content; + // override default ul styles + margin: 0; + padding: 0; .mx_IconizedContextMenu_optionList { & > * { diff --git a/src/components/structures/TabbedView.tsx b/src/components/structures/TabbedView.tsx index 5e79ffc1f3..b6369466fe 100644 --- a/src/components/structures/TabbedView.tsx +++ b/src/components/structures/TabbedView.tsx @@ -121,6 +121,7 @@ export default class TabbedView extends React.Component {tabIcon} @@ -166,14 +167,14 @@ export default class TabbedView extends React.Component {({ onKeyDownHandler }) => ( -
{labels} -
+ )} {panel} diff --git a/src/components/views/context_menus/IconizedContextMenu.tsx b/src/components/views/context_menus/IconizedContextMenu.tsx index b81f08f7bb..50913c24b0 100644 --- a/src/components/views/context_menus/IconizedContextMenu.tsx +++ b/src/components/views/context_menus/IconizedContextMenu.tsx @@ -126,6 +126,7 @@ export const IconizedContextMenuOption: React.FC = ({ }) => { return ( > = ({ classN return ( -
{children}
+
    + {children} +
); }; diff --git a/test/components/structures/__snapshots__/TabbedView-test.tsx.snap b/test/components/structures/__snapshots__/TabbedView-test.tsx.snap index acd6a4bae6..fc2b259aaa 100644 --- a/test/components/structures/__snapshots__/TabbedView-test.tsx.snap +++ b/test/components/structures/__snapshots__/TabbedView-test.tsx.snap @@ -5,12 +5,12 @@ exports[` renders tabs 1`] = `
-
-
renders tabs 1`] = ` > General -
-
+
  • renders tabs 1`] = ` > Labs -
  • -
    +
  • renders tabs 1`] = ` > Security -
  • -
    + +
    { createMenu(event, {}, {}, undefined, room); - expect(document.querySelector('div[aria-label="Pin"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Pin"]')).toBeFalsy(); }); it("does not show pin option for beacon_info event", () => { @@ -111,7 +111,7 @@ describe("MessageContextMenu", () => { createMenu(deadBeaconEvent, {}, {}, undefined, room); - expect(document.querySelector('div[aria-label="Pin"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Pin"]')).toBeFalsy(); }); it("does not show pin option when pinning feature is disabled", () => { @@ -130,7 +130,7 @@ describe("MessageContextMenu", () => { createMenu(pinnableEvent, {}, {}, undefined, room); - expect(document.querySelector('div[aria-label="Pin"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Pin"]')).toBeFalsy(); }); it("shows pin option when pinning feature is enabled", () => { @@ -147,7 +147,7 @@ describe("MessageContextMenu", () => { createMenu(pinnableEvent, {}, {}, undefined, room); - expect(document.querySelector('div[aria-label="Pin"]')).toBeTruthy(); + expect(document.querySelector('li[aria-label="Pin"]')).toBeTruthy(); }); it("pins event on pin option click", () => { @@ -171,7 +171,7 @@ describe("MessageContextMenu", () => { createMenu(pinnableEvent, { onFinished }, {}, undefined, room); - fireEvent.click(document.querySelector('div[aria-label="Pin"]')!); + fireEvent.click(document.querySelector('li[aria-label="Pin"]')!); // added to account data expect(client.setRoomAccountData).toHaveBeenCalledWith(roomId, ReadPinsEventId, { @@ -225,7 +225,7 @@ describe("MessageContextMenu", () => { createMenu(pinnableEvent, {}, {}, undefined, room); - fireEvent.click(document.querySelector('div[aria-label="Unpin"]')!); + fireEvent.click(document.querySelector('li[aria-label="Unpin"]')!); expect(client.setRoomAccountData).not.toHaveBeenCalled(); @@ -244,13 +244,13 @@ describe("MessageContextMenu", () => { it("allows forwarding a room message", () => { const eventContent = createMessageEventContent("hello"); createMenuWithContent(eventContent); - expect(document.querySelector('div[aria-label="Forward"]')).toBeTruthy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeTruthy(); }); it("does not allow forwarding a poll", () => { const eventContent = PollStartEvent.from("why?", ["42"], M_POLL_KIND_DISCLOSED); createMenuWithContent(eventContent); - expect(document.querySelector('div[aria-label="Forward"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeFalsy(); }); it("should not allow forwarding a voice broadcast", () => { @@ -261,7 +261,7 @@ describe("MessageContextMenu", () => { "ABC123", ); createMenu(broadcastStartEvent); - expect(document.querySelector('div[aria-label="Forward"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeFalsy(); }); describe("forwarding beacons", () => { @@ -273,7 +273,7 @@ describe("MessageContextMenu", () => { const beacons = new Map(); beacons.set(getBeaconInfoIdentifier(deadBeaconEvent), beacon); createMenu(deadBeaconEvent, {}, {}, beacons); - expect(document.querySelector('div[aria-label="Forward"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeFalsy(); }); it("does not allow forwarding a beacon that is not live but has a latestLocation", () => { @@ -288,7 +288,7 @@ describe("MessageContextMenu", () => { const beacons = new Map(); beacons.set(getBeaconInfoIdentifier(deadBeaconEvent), beacon); createMenu(deadBeaconEvent, {}, {}, beacons); - expect(document.querySelector('div[aria-label="Forward"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeFalsy(); }); it("does not allow forwarding a live beacon that does not have a latestLocation", () => { @@ -298,7 +298,7 @@ describe("MessageContextMenu", () => { const beacons = new Map(); beacons.set(getBeaconInfoIdentifier(beaconEvent), beacon); createMenu(beaconEvent, {}, {}, beacons); - expect(document.querySelector('div[aria-label="Forward"]')).toBeFalsy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeFalsy(); }); it("allows forwarding a live beacon that has a location", () => { @@ -313,7 +313,7 @@ describe("MessageContextMenu", () => { const beacons = new Map(); beacons.set(getBeaconInfoIdentifier(liveBeaconEvent), beacon); createMenu(liveBeaconEvent, {}, {}, beacons); - expect(document.querySelector('div[aria-label="Forward"]')).toBeTruthy(); + expect(document.querySelector('li[aria-label="Forward"]')).toBeTruthy(); }); it("opens forward dialog with correct event", () => { @@ -330,7 +330,7 @@ describe("MessageContextMenu", () => { beacons.set(getBeaconInfoIdentifier(liveBeaconEvent), beacon); createMenu(liveBeaconEvent, {}, {}, beacons); - fireEvent.click(document.querySelector('div[aria-label="Forward"]')!); + fireEvent.click(document.querySelector('li[aria-label="Forward"]')!); // called with forwardableEvent, not beaconInfo event expect(dispatchSpy).toHaveBeenCalledWith( @@ -395,7 +395,7 @@ describe("MessageContextMenu", () => { mocked(getSelectedText).mockReturnValue(text); createRightClickMenuWithContent(eventContent); - const copyButton = document.querySelector('div[aria-label="Copy"]')!; + const copyButton = document.querySelector('li[aria-label="Copy"]')!; fireEvent.mouseDown(copyButton); expect(copyPlaintext).toHaveBeenCalledWith(text); }); @@ -406,7 +406,7 @@ describe("MessageContextMenu", () => { mocked(getSelectedText).mockReturnValue(""); createRightClickMenuWithContent(eventContent); - const copyButton = document.querySelector('div[aria-label="Copy"]'); + const copyButton = document.querySelector('li[aria-label="Copy"]'); expect(copyButton).toBeFalsy(); }); @@ -415,7 +415,7 @@ describe("MessageContextMenu", () => { mocked(canEditContent).mockReturnValue(true); createRightClickMenuWithContent(eventContent); - const editButton = document.querySelector('div[aria-label="Edit"]'); + const editButton = document.querySelector('li[aria-label="Edit"]'); expect(editButton).toBeTruthy(); }); @@ -424,7 +424,7 @@ describe("MessageContextMenu", () => { mocked(canEditContent).mockReturnValue(false); createRightClickMenuWithContent(eventContent); - const editButton = document.querySelector('div[aria-label="Edit"]'); + const editButton = document.querySelector('li[aria-label="Edit"]'); expect(editButton).toBeFalsy(); }); @@ -435,7 +435,7 @@ describe("MessageContextMenu", () => { }; createRightClickMenuWithContent(eventContent, context); - const replyButton = document.querySelector('div[aria-label="Reply"]'); + const replyButton = document.querySelector('li[aria-label="Reply"]'); expect(replyButton).toBeTruthy(); }); @@ -449,7 +449,7 @@ describe("MessageContextMenu", () => { unsentMessage.setStatus(EventStatus.QUEUED); createMenu(unsentMessage, {}, context); - const replyButton = document.querySelector('div[aria-label="Reply"]'); + const replyButton = document.querySelector('li[aria-label="Reply"]'); expect(replyButton).toBeFalsy(); }); @@ -460,7 +460,7 @@ describe("MessageContextMenu", () => { }; createRightClickMenuWithContent(eventContent, context); - const reactButton = document.querySelector('div[aria-label="React"]'); + const reactButton = document.querySelector('li[aria-label="React"]'); expect(reactButton).toBeTruthy(); }); @@ -471,7 +471,7 @@ describe("MessageContextMenu", () => { }; createRightClickMenuWithContent(eventContent, context); - const reactButton = document.querySelector('div[aria-label="React"]'); + const reactButton = document.querySelector('li[aria-label="React"]'); expect(reactButton).toBeFalsy(); }); @@ -487,7 +487,7 @@ describe("MessageContextMenu", () => { }; createMenu(mxEvent, props, context); - const reactButton = document.querySelector('div[aria-label="View in room"]'); + const reactButton = document.querySelector('li[aria-label="View in room"]'); expect(reactButton).toBeTruthy(); }); @@ -495,7 +495,7 @@ describe("MessageContextMenu", () => { const eventContent = createMessageEventContent("hello"); createRightClickMenuWithContent(eventContent); - const reactButton = document.querySelector('div[aria-label="View in room"]'); + const reactButton = document.querySelector('li[aria-label="View in room"]'); expect(reactButton).toBeFalsy(); }); @@ -511,7 +511,7 @@ describe("MessageContextMenu", () => { createRightClickMenu(mxEvent, context); - const replyInThreadButton = document.querySelector('div[aria-label="Reply in thread"]')!; + const replyInThreadButton = document.querySelector('li[aria-label="Reply in thread"]')!; fireEvent.click(replyInThreadButton); expect(dispatcher.dispatch).toHaveBeenCalledWith({ diff --git a/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap b/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap index c1c49e5666..be217b26f2 100644 --- a/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap +++ b/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap @@ -16,8 +16,9 @@ exports[`RoomGeneralContextMenu renders an empty context menu for archived rooms
    -
    - +
    -
    +
    @@ -63,8 +64,9 @@ exports[`RoomGeneralContextMenu renders the default context menu 1`] = `
    -
    - +
    -
    +
    diff --git a/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap b/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap index bd732f4426..3693f9a5f1 100644 --- a/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap +++ b/test/components/views/context_menus/__snapshots__/SpaceContextMenu-test.tsx.snap @@ -16,8 +16,9 @@ exports[` renders menu correctly 1`] = ` class="mx_ContextualMenu" role="menu" > -   renders menu correctly 1`] = `  -  renders menu correctly 1`] = ` > Space home 
     -  -  +  renders menu correctly 1`] = ` > Explore rooms  -  -  +  renders menu correctly 1`] = ` > Preferences  -  -  +  renders menu correctly 1`] = ` > Leave space  -  +   -  +     diff --git a/test/components/views/dialogs/__snapshots__/RoomSettingsDialog-test.tsx.snap b/test/components/views/dialogs/__snapshots__/RoomSettingsDialog-test.tsx.snap index 1cb2be57d3..6c148400d8 100644 --- a/test/components/views/dialogs/__snapshots__/RoomSettingsDialog-test.tsx.snap +++ b/test/components/views/dialogs/__snapshots__/RoomSettingsDialog-test.tsx.snap @@ -2,7 +2,7 @@ exports[` Settings tabs renders default tabs correctly 1`] = ` NodeList [ -
    General -
    , -
    , +
  • Security & Privacy -
  • , -
    , +
  • Roles & Permissions -
  • , -
    , +
  • Notifications -
  • , -
    , +
  • Poll history -
  • , + , ] `; diff --git a/test/components/views/dialogs/__snapshots__/UserSettingsDialog-test.tsx.snap b/test/components/views/dialogs/__snapshots__/UserSettingsDialog-test.tsx.snap index 4c1e9b20f9..22d10366f2 100644 --- a/test/components/views/dialogs/__snapshots__/UserSettingsDialog-test.tsx.snap +++ b/test/components/views/dialogs/__snapshots__/UserSettingsDialog-test.tsx.snap @@ -2,7 +2,7 @@ exports[` renders tabs correctly 1`] = ` NodeList [ -
    General -
    , -
    , +
  • Appearance -
  • , -
    , +
  • Notifications -
  • , -
    , +
  • Preferences -
  • , -
    , +
  • Keyboard -
  • , -
    , +
  • Sidebar -
  • , -
    , +
  • Security & Privacy -
  • , -
    , +
  • Labs -
  • , -
    , +
  • Help & About -
  • , + , ] `;