Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make highlight Actions menu items (Edit and Delete) buttons #2332

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7a3b30b
Make highlight Actions menu items (Edit and Delete) buttons
RoyEJohnson Aug 21, 2024
b37519a
Fix styling in MH
RoyEJohnson Aug 27, 2024
d1656db
Focus on first item in revealed dropdown
RoyEJohnson Aug 28, 2024
6e21552
Focus follows mouse
RoyEJohnson Aug 28, 2024
ae539c0
Style focus, not hover
RoyEJohnson Sep 4, 2024
7e8a6d4
Focus on DisplayNote goes to the menu button
RoyEJohnson Sep 18, 2024
dd06dbe
Revert "Focus on DisplayNote goes to the menu button"
RoyEJohnson Oct 2, 2024
5ca988a
Use aria-labelledby for Display note so the note is the label
RoyEJohnson Oct 2, 2024
03969dc
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 5, 2024
8e5334a
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
71305aa
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
faee922
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
c0ef090
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
4531929
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
15441c0
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
4e07c05
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
b78c584
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
4d3858f
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
5c30d06
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 10, 2024
45504a6
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 15, 2024
55a277c
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Oct 31, 2024
9190716
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
686adc8
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
d3da0f7
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
bf15489
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
f4591d1
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
dcefaf2
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 4, 2024
db45b81
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 5, 2024
6224d16
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 6, 2024
9fd9fe8
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 13, 2024
d718807
Merge branch 'main' into focus-on-first-item-in-MH-dialog
staxly[bot] Nov 26, 2024
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
15 changes: 12 additions & 3 deletions src/app/components/Dropdown.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ describe('Dropdown', () => {
const useOnEscSpy = jest.spyOn(reactUtils, 'useOnEsc');

const focus = jest.fn();
const focus2 = jest.fn();
const addEventListener = jest.fn();
const removeEventListener = jest.fn();
const createNodeMock = () => ({focus, addEventListener, removeEventListener});
Expand All @@ -126,12 +127,19 @@ describe('Dropdown', () => {

expect(() => component.root.findByType(DropdownList)).not.toThrow();

renderer.act(() => {
const buttons = component.root.findAllByType('button');

buttons[1].props.onMouseEnter({target: {focus: focus2}});
});

renderer.act(() => {
useOnEscSpy.mock.calls[0][1]();
});

expect(() => component.root.findByType(DropdownList)).toThrow();
expect(focus).toHaveBeenCalled();
expect(focus2).toHaveBeenCalled();

useOnEscSpy.mockClear();
});
Expand All @@ -149,9 +157,10 @@ describe('Dropdown', () => {
</TestContainer>);

renderer.act(() => {
const [button1, button2] = component.root.findAllByType('a');
button1.props.onClick(mockEv);
button2.props.onClick(mockEv);
const items = component.root.findAll((i) => i.props.onClick && i.type === 'button');

items.forEach((i) => i.props.onClick(mockEv));
expect(items.length).toBe(2);
});

expect(mockEv.preventDefault).toHaveBeenCalledTimes(2);
Expand Down
57 changes: 37 additions & 20 deletions src/app/components/Dropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@

import { HTMLElement } from '@openstax/types/lib.dom';
import { HTMLElement, HTMLMenuElement } from '@openstax/types/lib.dom';
import flow from 'lodash/fp/flow';
import isUndefined from 'lodash/fp/isUndefined';
import omitBy from 'lodash/fp/omitBy';
import React, { ReactNode } from 'react';
import { FormattedMessage, useIntl } from 'react-intl';
import styled, { css, keyframes } from 'styled-components/macro';
import { useFocusLost, useTrapTabNavigation } from '../reactUtils';
import { useFocusLost, useTrapTabNavigation, focusableItemQuery } from '../reactUtils';
import { useOnEsc } from '../reactUtils';
import theme, { defaultFocusOutline } from '../theme';
import { preventDefault } from '../utils';
Expand Down Expand Up @@ -167,18 +167,28 @@ const TabTransparentDropdown = styled((
`;

function TrappingDropdownList(props: object) {
const ref = React.useRef(null);
const ref = React.useRef<HTMLMenuElement>(null);

useTrapTabNavigation(ref);

React.useEffect(
() => {
if (ref.current?.querySelector) {
ref.current?.querySelector<HTMLElement>(focusableItemQuery)?.focus();
}
},
[]
);

return (
<ol ref={ref} {...props} />
<menu ref={ref} {...props} />
);
}


// tslint:disable-next-line:variable-name
export const DropdownList = styled(TrappingDropdownList)`
list-style: none;
margin: 0;
padding: 0.6rem 0;
background: ${theme.color.neutral.formBackground};
Expand Down Expand Up @@ -207,7 +217,6 @@ export const DropdownList = styled(TrappingDropdownList)`
font-size: 1.4rem;
line-height: 2rem;

&:hover,
&:focus {
background: ${theme.color.neutral.formBorder};
${defaultFocusOutline}
Expand Down Expand Up @@ -235,24 +244,32 @@ const DropdownItemContent = ({
'data-analytics-label': dataAnalyticsLabel,
'data-analytics-region': dataAnalyticsRegion,
});
return <FormattedMessage id={message}>
const focusMe = React.useCallback(
({target: me}) => me.focus(),
[]
);

return <FormattedMessage id={message}>
{(msg) => href
? <a href={href} tabIndex={0} onClick={onClick} target={target} {...analyticsDataProps}>{prefix}{msg}</a>
/*
this should be a button but Safari and firefox don't support focusing buttons
which breaks the tab transparent dropdown
https://bugs.webkit.org/show_bug.cgi?id=22261
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
*/
// eslint-disable-next-line jsx-a11y/anchor-is-valid
: <a
? <a
role='button'
href={href}
tabIndex={0}
onClick={onClick}
target={target}
onMouseEnter={focusMe}
{...analyticsDataProps}
>{prefix}{msg}</a>
// Safari support tab-navigation of buttons; this operates with space or Enter
: <button
type='button'
tabIndex={0}
href=''
onClick={onClick ? flow(preventDefault, onClick) : preventDefault}
onMouseEnter={focusMe}
{...analyticsDataProps}
>
{prefix}{msg}
</a>
</button>
}
</FormattedMessage>;
};
Expand All @@ -261,9 +278,9 @@ const DropdownItemContent = ({
export const DropdownItem = ({ariaMessage, ...contentProps}: DropdownItemProps) => {
const intl = useIntl();

return ariaMessage
? <li aria-label={intl.formatMessage({id: ariaMessage})}><DropdownItemContent {...contentProps}/></li>
: <li><DropdownItemContent {...contentProps} /></li>;
return <li aria-label={ariaMessage ? intl.formatMessage({id: ariaMessage}) : undefined}>
<DropdownItemContent {...contentProps}/>
</li>;
};

interface CommonDropdownProps {
Expand Down
32 changes: 18 additions & 14 deletions src/app/components/__snapshots__/DotMenu.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ exports[`Dropdown matches snapshot 1`] = `
}

.c12 {
list-style: none;
margin: 0;
padding: 0.6rem 0;
background: #f5f5f5;
Expand Down Expand Up @@ -173,8 +174,6 @@ exports[`Dropdown matches snapshot 1`] = `
line-height: 2rem;
}

.c12 li button:hover,
.c12 li a:hover,
.c12 li button:focus,
.c12 li a:focus {
background: #d5d5d5;
Expand Down Expand Up @@ -224,28 +223,31 @@ exports[`Dropdown matches snapshot 1`] = `
</svg>
</div>
</button>
<ol
<menu
className="c12"
>
<li>
<a
href=""
<button
onClick={[Function]}
onMouseEnter={[Function]}
tabIndex={0}
type="button"
>
Delete
</a>
</button>
</li>
<li>
<a
href="/wooo"
onClick={[Function]}
onMouseEnter={[Function]}
role="button"
tabIndex={0}
>
Edit
</a>
</li>
</ol>
</menu>
</div>
<button
aria-label="Actions"
Expand Down Expand Up @@ -409,6 +411,7 @@ exports[`Dropdown matches snapshot on right align 1`] = `
}

.c12 {
list-style: none;
margin: 0;
padding: 0.6rem 0;
background: #f5f5f5;
Expand Down Expand Up @@ -446,8 +449,6 @@ exports[`Dropdown matches snapshot on right align 1`] = `
line-height: 2rem;
}

.c12 li button:hover,
.c12 li a:hover,
.c12 li button:focus,
.c12 li a:focus {
background: #d5d5d5;
Expand Down Expand Up @@ -497,29 +498,32 @@ exports[`Dropdown matches snapshot on right align 1`] = `
</svg>
</div>
</button>
<ol
<menu
className="c12"
rightAlign={true}
>
<li>
<a
href=""
<button
onClick={[Function]}
onMouseEnter={[Function]}
tabIndex={0}
type="button"
>
Delete
</a>
</button>
</li>
<li>
<a
href="/wooo"
onClick={[Function]}
onMouseEnter={[Function]}
role="button"
tabIndex={0}
>
Edit
</a>
</li>
</ol>
</menu>
</div>
<button
aria-label="Actions"
Expand Down
32 changes: 18 additions & 14 deletions src/app/components/__snapshots__/Dropdown.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ exports[`Dropdown matches snapshot 1`] = `
}

.c6 {
list-style: none;
margin: 0;
padding: 0.6rem 0;
background: #f5f5f5;
Expand Down Expand Up @@ -129,8 +130,6 @@ exports[`Dropdown matches snapshot 1`] = `
line-height: 2rem;
}

.c6 li button:hover,
.c6 li a:hover,
.c6 li button:focus,
.c6 li a:focus {
background: #d5d5d5;
Expand All @@ -155,28 +154,31 @@ exports[`Dropdown matches snapshot 1`] = `
>
show more
</button>
<ol
<menu
className="c6"
>
<li>
<a
href=""
<button
onClick={[Function]}
onMouseEnter={[Function]}
tabIndex={0}
type="button"
>
Delete
</a>
</button>
</li>
<li>
<a
href="/wooo"
onClick={[Function]}
onMouseEnter={[Function]}
role="button"
tabIndex={0}
>
Edit
</a>
</li>
</ol>
</menu>
</div>
<button
className="c4 c5"
Expand Down Expand Up @@ -400,6 +402,7 @@ exports[`Dropdown matches snapshot for tab hidden (open) 1`] = `
}

.c4 {
list-style: none;
margin: 0;
padding: 0.6rem 0;
background: #f5f5f5;
Expand Down Expand Up @@ -437,8 +440,6 @@ exports[`Dropdown matches snapshot for tab hidden (open) 1`] = `
line-height: 2rem;
}

.c4 li button:hover,
.c4 li a:hover,
.c4 li button:focus,
.c4 li a:focus {
background: #d5d5d5;
Expand All @@ -461,27 +462,30 @@ exports[`Dropdown matches snapshot for tab hidden (open) 1`] = `
>
show more
</button>
<ol
<menu
className="c4"
>
<li>
<a
href=""
<button
onClick={[Function]}
onMouseEnter={[Function]}
tabIndex={0}
type="button"
>
Delete
</a>
</button>
</li>
<li>
<a
href="/wooo"
onClick={[Function]}
onMouseEnter={[Function]}
role="button"
tabIndex={0}
>
Edit
</a>
</li>
</ol>
</menu>
</div>
`;
Loading
Loading