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

Use semantic list elements for menu lists and tab lists #10902

Merged
merged 4 commits into from
May 15, 2023
Merged

Use semantic list elements for menu lists and tab lists #10902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e297733
Use semantic list elements for pop up menu lists
t3chguy May 15, 2023
dcd9993
Use semantic list elements for tab lists
t3chguy May 15, 2023
ca0a2df
Fix tests
t3chguy May 15, 2023
19e02a3
Update snapshot
t3chguy May 15, 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
3 changes: 3 additions & 0 deletions res/css/views/context_menus/_IconizedContextMenu.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
& > * {
Expand Down
5 changes: 3 additions & 2 deletions src/components/structures/TabbedView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default class TabbedView<T extends string> extends React.Component<IProps
role="tab"
aria-selected={isActive}
aria-controls={id}
element="li"
>
{tabIcon}
<span className="mx_TabbedView_tabLabel_text" id={`${id}_label`}>
Expand Down Expand Up @@ -166,14 +167,14 @@ export default class TabbedView<T extends string> extends React.Component<IProps
handleUpDown={this.props.tabLocation == TabLocation.LEFT}
>
{({ onKeyDownHandler }) => (
<div
<ul
className="mx_TabbedView_tabLabels"
role="tablist"
aria-orientation={this.props.tabLocation == TabLocation.LEFT ? "vertical" : "horizontal"}
onKeyDown={onKeyDownHandler}
>
{labels}
</div>
</ul>
)}
</RovingTabIndexProvider>
{panel}
Expand Down
5 changes: 4 additions & 1 deletion src/components/views/context_menus/IconizedContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export const IconizedContextMenuOption: React.FC<IOptionProps> = ({
}) => {
return (
<MenuItem
element="li"
{...props}
className={classNames(className, {
mx_IconizedContextMenu_item: true,
Expand Down Expand Up @@ -171,7 +172,9 @@ const IconizedContextMenu: React.FC<React.PropsWithChildren<IProps>> = ({ classN

return (
<ContextMenu chevronFace={ChevronFace.None} {...props}>
<div className={classes}>{children}</div>
<ul role="none" className={classes}>
{children}
</ul>
</ContextMenu>
);
};
Expand Down
16 changes: 8 additions & 8 deletions test/components/structures/__snapshots__/TabbedView-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ exports[`<TabbedView /> renders tabs 1`] = `
<div
class="mx_TabbedView mx_TabbedView_tabsOnLeft"
>
<div
<ul
aria-orientation="vertical"
class="mx_TabbedView_tabLabels"
role="tablist"
>
<div
<li
aria-controls="mx_tabpanel_GENERAL"
aria-selected="true"
class="mx_AccessibleButton mx_TabbedView_tabLabel mx_TabbedView_tabLabel_active"
Expand All @@ -27,8 +27,8 @@ exports[`<TabbedView /> renders tabs 1`] = `
>
General
</span>
</div>
<div
</li>
<li
aria-controls="mx_tabpanel_LABS"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
Expand All @@ -45,8 +45,8 @@ exports[`<TabbedView /> renders tabs 1`] = `
>
Labs
</span>
</div>
<div
</li>
<li
aria-controls="mx_tabpanel_SECURITY"
aria-selected="false"
class="mx_AccessibleButton mx_TabbedView_tabLabel"
Expand All @@ -63,8 +63,8 @@ exports[`<TabbedView /> renders tabs 1`] = `
>
Security
</span>
</div>
</div>
</li>
</ul>
<div
aria-labelledby="mx_tabpanel_GENERAL_label"
class="mx_TabbedView_tabPanel"
Expand Down
50 changes: 25 additions & 25 deletions test/components/views/context_menus/MessageContextMenu-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe("MessageContextMenu", () => {

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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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, {
Expand Down Expand Up @@ -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();

Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -273,7 +273,7 @@ describe("MessageContextMenu", () => {
const beacons = new Map<BeaconIdentifier, Beacon>();
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", () => {
Expand All @@ -288,7 +288,7 @@ describe("MessageContextMenu", () => {
const beacons = new Map<BeaconIdentifier, Beacon>();
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", () => {
Expand All @@ -298,7 +298,7 @@ describe("MessageContextMenu", () => {
const beacons = new Map<BeaconIdentifier, Beacon>();
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", () => {
Expand All @@ -313,7 +313,7 @@ describe("MessageContextMenu", () => {
const beacons = new Map<BeaconIdentifier, Beacon>();
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", () => {
Expand All @@ -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(
Expand Down Expand Up @@ -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);
});
Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -487,15 +487,15 @@ 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();
});

it("does not show view in room button when the event is not a thread root", () => {
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();
});

Expand All @@ -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({
Expand Down
18 changes: 10 additions & 8 deletions test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,17 @@ exports[`RoomGeneralContextMenu renders an empty context menu for archived rooms
<div
class="mx_ContextualMenu_chevron_left"
/>
<div
<ul
class="mx_IconizedContextMenu mx_RoomGeneralContextMenu mx_IconizedContextMenu_compact"
role="none"
>
<div
class="mx_IconizedContextMenu_optionList mx_IconizedContextMenu_optionList_notFirst"
/>
<div
class="mx_IconizedContextMenu_optionList mx_IconizedContextMenu_optionList_notFirst mx_IconizedContextMenu_optionList_red"
>
<div
<li
aria-label="Forget Room"
class="mx_AccessibleButton mx_IconizedContextMenu_option_red mx_IconizedContextMenu_item"
role="menuitem"
Expand All @@ -39,9 +40,9 @@ exports[`RoomGeneralContextMenu renders an empty context menu for archived rooms
>
Forget Room
</span>
</div>
</li>
</div>
</div>
</ul>
</div>
</div>
</div>
Expand All @@ -63,16 +64,17 @@ exports[`RoomGeneralContextMenu renders the default context menu 1`] = `
<div
class="mx_ContextualMenu_chevron_left"
/>
<div
<ul
class="mx_IconizedContextMenu mx_RoomGeneralContextMenu mx_IconizedContextMenu_compact"
role="none"
>
<div
class="mx_IconizedContextMenu_optionList mx_IconizedContextMenu_optionList_notFirst"
/>
<div
class="mx_IconizedContextMenu_optionList mx_IconizedContextMenu_optionList_notFirst mx_IconizedContextMenu_optionList_red"
>
<div
<li
aria-label="Forget Room"
class="mx_AccessibleButton mx_IconizedContextMenu_option_red mx_IconizedContextMenu_item"
role="menuitem"
Expand All @@ -86,9 +88,9 @@ exports[`RoomGeneralContextMenu renders the default context menu 1`] = `
>
Forget Room
</span>
</div>
</li>
</div>
</div>
</ul>
</div>
</div>
</div>
Expand Down
Loading