Commit 5cce2e59 authored by Alejandro Celaya's avatar Alejandro Celaya Committed by Alejandro Celaya

Make FileInput a single Button to improve accessibility

parent cef29a96
import { import { Button, FileGenericIcon } from '@hypothesis/frontend-shared';
FileGenericIcon, import classnames from 'classnames';
IconButton, import { useRef, useState } from 'preact/hooks';
Input,
InputGroup,
} from '@hypothesis/frontend-shared';
import { useId, useRef } from 'preact/hooks';
export type FileInputProps = { export type FileInputProps = {
onFileSelected: (file: File) => void; onFileSelected: (file: File) => void;
...@@ -16,10 +12,20 @@ export default function FileInput({ ...@@ -16,10 +12,20 @@ export default function FileInput({
disabled, disabled,
}: FileInputProps) { }: FileInputProps) {
const fileInputRef = useRef<HTMLInputElement>(null); const fileInputRef = useRef<HTMLInputElement>(null);
const inputId = useId(); const [filename, setFilename] = useState<string | null>(null);
return ( return (
<> <Button
variant="custom"
classes={classnames(
'w-full relative overflow-hidden border rounded',
'bg-grey-0 hover:bg-grey-0',
)}
title={filename ? `${filename}, Select a file` : 'Select a file'}
onClick={() => fileInputRef.current?.click()}
disabled={disabled}
data-testid="file-input-proxy-button"
>
<input <input
ref={fileInputRef} ref={fileInputRef}
accept=".json" accept=".json"
...@@ -30,31 +36,22 @@ export default function FileInput({ ...@@ -30,31 +36,22 @@ export default function FileInput({
data-testid="file-input" data-testid="file-input"
onChange={e => { onChange={e => {
const files = (e.target as HTMLInputElement)!.files; const files = (e.target as HTMLInputElement)!.files;
setFilename(files?.[0]?.name ?? null);
if (files !== null && files.length > 0) { if (files !== null && files.length > 0) {
onFileSelected(files[0]); onFileSelected(files[0]);
} }
}} }}
/> />
<label htmlFor={inputId}>Select Hypothesis export file:</label> <div
<InputGroup> className="max-w-full pr-10 truncate"
<Input data-testid="filename-container"
id={inputId} >
onClick={() => fileInputRef.current?.click()} {filename ?? 'Select a file'}
readonly </div>
disabled={disabled} <div className="absolute right-0 h-full p-2 border-l bg-grey-2">
value={fileInputRef.current?.files![0]?.name ?? 'Select a file'} <FileGenericIcon />
data-testid="file-input-proxy-input" </div>
classes="cursor-pointer" </Button>
/>
<IconButton
title="Select a file"
onClick={() => fileInputRef.current?.click()}
icon={FileGenericIcon}
variant="dark"
disabled={disabled}
data-testid="file-input-proxy-button"
/>
</InputGroup>
</>
); );
} }
...@@ -143,6 +143,7 @@ function ImportAnnotations({ ...@@ -143,6 +143,7 @@ function ImportAnnotations({
return ( return (
<> <>
<p>Select Hypothesis export file:</p>
<FileInput onFileSelected={setFile} disabled={busy} /> <FileInput onFileSelected={setFile} disabled={busy} />
{userList && ( {userList && (
<> <>
......
...@@ -29,10 +29,10 @@ describe('FileInput', () => { ...@@ -29,10 +29,10 @@ describe('FileInput', () => {
const getActualFileInput = wrapper => const getActualFileInput = wrapper =>
wrapper.find('[data-testid="file-input"]'); wrapper.find('[data-testid="file-input"]');
const getProxyInput = wrapper =>
wrapper.find('input[data-testid="file-input-proxy-input"]');
const getProxyButton = wrapper => const getProxyButton = wrapper =>
wrapper.find('button[data-testid="file-input-proxy-button"]'); wrapper.find('button[data-testid="file-input-proxy-button"]');
const getFilenameContainer = wrapper =>
wrapper.find('[data-testid="filename-container"]');
const createInput = (disabled = undefined) => { const createInput = (disabled = undefined) => {
const wrapper = mount( const wrapper = mount(
...@@ -52,10 +52,14 @@ describe('FileInput', () => { ...@@ -52,10 +52,14 @@ describe('FileInput', () => {
const firstFile = createFile('foo'); const firstFile = createFile('foo');
const fileInput = getActualFileInput(wrapper); const fileInput = getActualFileInput(wrapper);
// We display a placeholder/CTA before any file has been selected
assert.equal(getFilenameContainer(wrapper).text(), 'Select a file');
fillInputWithFiles(fileInput, [firstFile, createFile('bar')]); fillInputWithFiles(fileInput, [firstFile, createFile('bar')]);
fileInput.simulate('change'); fileInput.simulate('change');
assert.calledWith(fakeOnFileSelected, firstFile); assert.calledWith(fakeOnFileSelected, firstFile);
assert.equal(getFilenameContainer(wrapper).text(), 'foo.json');
}); });
it('does not call onFileSelected when input changes with no files', () => { it('does not call onFileSelected when input changes with no files', () => {
...@@ -67,15 +71,6 @@ describe('FileInput', () => { ...@@ -67,15 +71,6 @@ describe('FileInput', () => {
assert.notCalled(fakeOnFileSelected); assert.notCalled(fakeOnFileSelected);
}); });
it('forwards click on proxy input to actual file input', () => {
const wrapper = createInput();
const proxyInput = getProxyInput(wrapper);
proxyInput.simulate('click');
assert.called(getActualFileInput(wrapper).getDOMNode().click);
});
it('forwards click on proxy button to actual file input', () => { it('forwards click on proxy button to actual file input', () => {
const wrapper = createInput(); const wrapper = createInput();
const proxyButton = getProxyButton(wrapper); const proxyButton = getProxyButton(wrapper);
...@@ -89,11 +84,9 @@ describe('FileInput', () => { ...@@ -89,11 +84,9 @@ describe('FileInput', () => {
it('disables all inner components when FileInput is disabled', () => { it('disables all inner components when FileInput is disabled', () => {
const wrapper = createInput(disabled); const wrapper = createInput(disabled);
const fileInput = getActualFileInput(wrapper); const fileInput = getActualFileInput(wrapper);
const proxyInput = getProxyInput(wrapper);
const proxyButton = getProxyButton(wrapper); const proxyButton = getProxyButton(wrapper);
assert.equal(fileInput.prop('disabled'), disabled); assert.equal(fileInput.prop('disabled'), disabled);
assert.equal(proxyInput.prop('disabled'), disabled);
assert.equal(proxyButton.prop('disabled'), disabled); assert.equal(proxyButton.prop('disabled'), disabled);
}); });
}); });
...@@ -102,12 +95,7 @@ describe('FileInput', () => { ...@@ -102,12 +95,7 @@ describe('FileInput', () => {
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility([ checkAccessibility([
{ {
content: () => content: () => mount(<FileInput onFileSelected={fakeOnFileSelected} />),
mount(
<div>
<FileInput onFileSelected={fakeOnFileSelected} />
</div>,
),
}, },
]), ]),
); );
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment