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

Commit

Permalink
Use semantic list elements for menu lists and tab lists (#10902)
Browse files Browse the repository at this point in the history 
* Use semantic list elements for pop up menu lists

* Use semantic list elements for tab lists

* Fix tests

* Update snapshot
  • Loading branch information
@t3chguy
t3chguy authored May 15, 2023
1 parent 8b7eb8b commit 296ed22
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 82 deletions.
3 changes: 3 additions & 0 deletions res/css/views/context_menus/_IconizedContextMenu.pcss
View file
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
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
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
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
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
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

0 comments on commit 296ed22

Please sign in to comment.