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

Category widget UI: change focus order to increase keyboard accessibility #857

Merged
merged 12 commits into from
Mar 22, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Not released

- Category widget UI: increase keyboard accessibility [#856](https://github.com/CartoDB/carto-react/pull/856)
- Category widget UI: change focus order to increase keyboard accessibility [#857](https://github.com/CartoDB/carto-react/pull/857)

## 2.4

Expand Down
11 changes: 7 additions & 4 deletions packages/react-ui/__tests__/widgets/CategoryWidgetUI.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ describe('CategoryWidgetUI', () => {
});

describe('events', () => {
beforeEach(() => {
HTMLElement.prototype.scrollIntoView = jest.fn();
});

test('category change', () => {
const mockOnSelectedCategoriesChange = jest.fn();
render(
Expand Down Expand Up @@ -132,13 +136,12 @@ describe('CategoryWidgetUI', () => {

fireEvent.click(screen.getByText(/Search in 4 elements/));
fireEvent.click(screen.getByText(/Category 1/));
fireEvent.click(screen.getByText(/Apply/));
fireEvent.click(screen.getByTestId('primaryApplyButton'));
fireEvent.click(screen.getByText(/Unlock/));
expect(mockOnSelectedCategoriesChange).toHaveBeenCalledTimes(2);
});

test('search category', () => {
HTMLElement.prototype.scrollIntoView = jest.fn();
const mockOnSelectedCategoriesChange = jest.fn();
render(
<CategoryWidgetUI
Expand All @@ -151,15 +154,15 @@ describe('CategoryWidgetUI', () => {
fireEvent.click(screen.getByText(/Search in 4 elements/));
userEvent.type(screen.getByRole('textbox'), 'Category 1');
fireEvent.click(screen.getByText(/Category 1/));
fireEvent.click(screen.getByText(/Apply/));
fireEvent.click(screen.getByTestId('primaryApplyButton'));
});

test('cancel search', () => {
render(<CategoryWidgetUI data={DATA} maxItems={1} />);

expect(screen.getByText(/Search in 4 elements/)).toBeInTheDocument();
fireEvent.click(screen.getByText(/Search in 4 elements/));
fireEvent.click(screen.getByText(/Cancel/));
fireEvent.click(screen.getByTestId('primaryCancelButton'));
});

test('searchable prop', () => {
Expand Down
71 changes: 51 additions & 20 deletions packages/react-ui/src/widgets/CategoryWidgetUI/CategoryWidgetUI.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import {
LinkAsButton,
OptionsSelectedBar,
ProgressBar,
CategoriesRoot
CategoriesRoot,
CategoryLabelWrapper,
HiddenButton
} from './CategoryWidgetUI.styled';
import SearchIcon from '../../assets/icons/SearchIcon';
import useImperativeIntl from '../../hooks/useImperativeIntl';
Expand Down Expand Up @@ -60,6 +62,7 @@ function CategoryWidgetUI(props) {
const [tempBlockedCategories, setTempBlockedCategories] = useState(false);
const [animValues, setAnimValues] = useState([]);
const requestRef = useRef();
const searchRef = useRef();
const prevAnimValues = usePrevious(animValues);
const referencedPrevAnimValues = useRef();
const { showSkeleton } = useSkeleton(isLoading);
Expand Down Expand Up @@ -303,6 +306,12 @@ function CategoryWidgetUI(props) {
}
}, [animation, sortedData]);

useEffect(() => {
if (showAll && searchRef.current) {
searchRef.current.focus();
}
}, [showAll, searchRef]);

// Separated to simplify the widget layout but inside the main component to avoid passing all dependencies
const CategoryItem = (props) => {
const { data, onCategoryClick } = props;
Expand Down Expand Up @@ -334,6 +343,7 @@ function CategoryWidgetUI(props) {
!showAll &&
selectedCategories.length > 0 &&
selectedCategories.indexOf(data.name) === -1;

return (
<CategoryItemGroup
container
Expand All @@ -347,14 +357,14 @@ function CategoryWidgetUI(props) {
tabIndex={filterable ? 0 : -1}
>
{filterable && showAll && (
<Grid item>
<Grid item mr={1}>
<Checkbox
checked={tempBlockedCategories.indexOf(data.name) !== -1}
tabIndex={-1}
/>
</Grid>
)}
<Grid container item xs>
<CategoryLabelWrapper container item xs isSelectable={showAll}>
<Grid
container
item
Expand Down Expand Up @@ -383,7 +393,7 @@ function CategoryWidgetUI(props) {
<ProgressBar className='progressbar' item>
<div style={{ width: getProgressbarLength(data.value) }}></div>
</ProgressBar>
</Grid>
</CategoryLabelWrapper>
</CategoryItemGroup>
);
};
Expand All @@ -408,6 +418,7 @@ function CategoryWidgetUI(props) {
onKeyDown={handleApplyPress}
underline='hover'
tabIndex={0}
data-testid='primaryApplyButton'
>
{intlConfig.formatMessage({ id: 'c4r.widgets.category.apply' })}
</LinkAsButton>
Expand Down Expand Up @@ -450,7 +461,9 @@ function CategoryWidgetUI(props) {
<TextField
size='small'
mt={-0.5}
placeholder={intlConfig.formatMessage({ id: 'c4r.widgets.category.search' })}
placeholder={intlConfig.formatMessage({
id: 'c4r.widgets.category.search'
})}
onChange={handleSearchChange}
onFocus={handleSearchFocus}
InputProps={{
Expand All @@ -461,9 +474,13 @@ function CategoryWidgetUI(props) {
)
}}
inputProps={{
tabIndex: 0
tabIndex: 0,
ref: searchRef
}}
/>
<HiddenButton size='small' onClick={handleCancelClicked}>
{intlConfig.formatMessage({ id: 'c4r.widgets.category.cancel' })}
</HiddenButton>
</OptionsSelectedBar>
)}
<CategoriesWrapper container item>
Expand Down Expand Up @@ -491,23 +508,37 @@ function CategoryWidgetUI(props) {
</Box>
)}
</CategoriesWrapper>
{showAll && (
<HiddenButton size='small' onClick={handleApplyClicked}>
{intlConfig.formatMessage({ id: 'c4r.widgets.category.apply' })}
</HiddenButton>
)}
{data.length > maxItems && searchable ? (
showAll ? (
<Button size='small' color='primary' onClick={handleCancelClicked}>
{intlConfig.formatMessage({ id: 'c4r.widgets.category.cancel' })}
</Button>
<Box mt={1.5}>
<Button
size='small'
color='primary'
onClick={handleCancelClicked}
data-testid='primaryCancelButton'
>
{intlConfig.formatMessage({ id: 'c4r.widgets.category.cancel' })}
</Button>
</Box>
) : (
<Button
size='small'
color='primary'
startIcon={<SearchIcon />}
onClick={handleShowAllCategoriesClicked}
>
{intlConfig.formatMessage(
{ id: 'c4r.widgets.category.searchInfo' },
{ elements: getCategoriesCount() }
)}
</Button>
<Box mt={1.5}>
<Button
size='small'
color='primary'
startIcon={<SearchIcon />}
onClick={handleShowAllCategoriesClicked}
>
{intlConfig.formatMessage(
{ id: 'c4r.widgets.category.searchInfo' },
{ elements: getCategoriesCount() }
)}
</Button>
</Box>
)
) : null}
</CategoriesRoot>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,32 @@
import { Box, Grid, Link, styled } from '@mui/material';
import { Box, Button, Grid, Link, styled } from '@mui/material';
import Typography from '../../components/atoms/Typography';

const REST_CATEGORY = '__rest__';

export const CategoriesWrapper = styled(Grid)(({ theme: { spacing } }) => ({
maxHeight: spacing(40),
padding: spacing(0, 1, 1, 0)
export const CategoriesWrapper = styled(Grid)(({ theme }) => ({
maxHeight: theme.spacing(40),
overflow: 'auto',
padding: 0
}));

export const CategoryItemGroup = styled(Grid, {
shouldForwardProp: (prop) => !['selectable', 'name', 'unselected'].includes(prop)
})(({ theme, selectable, name, unselected }) => {
return {
flexDirection: 'row',
maxWidth: '100%',
padding: theme.spacing(0.5, 0.25),
margin: 0,

'> .MuiGrid-item': {
paddingTop: 0,
paddingLeft: 0
},
'&:focus-visible': {
outline: `none !important`,
boxShadow: `inset 0 0 0 2px ${theme.palette.primary.main} !important`
},

...(unselected && {
color: theme.palette.text.disabled,

Expand Down Expand Up @@ -54,7 +68,7 @@ export const OptionsSelectedBar = styled(Grid)(({ theme: { spacing, palette } })
export const ProgressBar = styled(Grid)(({ theme }) => ({
height: theme.spacing(0.5),
width: '100%',
margin: theme.spacing(0.5, 0, 1, 0),
margin: theme.spacing(0.5, 0, 0.25, 0),
borderRadius: theme.spacing(0.5),
backgroundColor: theme.palette.action.disabledBackground,

Expand All @@ -68,6 +82,16 @@ export const ProgressBar = styled(Grid)(({ theme }) => ({
}
}));

export const CategoryLabelWrapper = styled(Grid, {
shouldForwardProp: (prop) => prop !== 'isSelectable'
})(({ theme, isSelectable }) => {
return {
...(isSelectable && {
width: `calc(100% - ${theme.spacing(4)})`
})
};
});

export const CategoryLabel = styled(Typography)(({ theme }) => ({
fontWeight: theme.typography.fontWeightBold,
marginRight: theme.spacing(2)
Expand All @@ -85,3 +109,19 @@ export const LinkAsButton = styled(Link)(({ theme }) => ({
export const CategoriesRoot = styled(Box)(({ theme }) => ({
...theme.typography.body2
}));

export const HiddenButton = styled(Button)(({ theme }) => ({
position: 'absolute',
left: '-999px',
top: '-1px',
width: '1px',
height: '1px',
display: 'inline-flex',

'&:focus-visible': {
position: 'static',
width: 'auto',
height: 'auto',
marginTop: theme.spacing(2)
}
}));
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const LoadingTemplate = (args) => {
);
};

const data = [...Array(7)].map((_, idx) => ({
const data = [...Array(30)].map((_, idx) => ({
name: `Category ${idx + 1}`,
value: idx * 100
}));
Expand Down
Loading