-
Notifications
You must be signed in to change notification settings - Fork 336
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
Add progressively enhanced file upload component #5305
base: main
Are you sure you want to change the base?
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for 2e256f5 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
9113f76
to
26b05e1
Compare
26b05e1
to
8fe1f08
Compare
I've started testing this today. But I've only tested with Dragon so far. Unfortunately it doesn't work. Roughly 9 out of 10 times saying "click choose file" or "click button" only focused on the button but didn't open the file dialog. And weirdly 1 out of 10 times it did. But I didn't do anything differently between when it did and when it didn't. When we've figured out why this is happening I will test again, including in other assistive technologies. |
8fe1f08
to
d411a91
Compare
Rebased this whilst I do some local testing |
Just in case it's helpful - we had a styled upload button on the passport service - which has presumably been tested by DAC multiple times. |
@edwardhorsford Do you know if it was tested in Dragon NaturallySpeaking and, if so, whether it exhibited the same issues described by @selfthinker? |
I assume it was tested with it as DAC did the testing. But I don't recall more than that. You could try on the live service to see how it works now... |
cee7848
to
9cfd3be
Compare
Adding
|
DAC's solution was using the |
6ab7ef8
to
f2bccb3
Compare
We check that `$root` is an input with `type="file"` so it will have a `files` property that's not null.
Remove comments ignoring TypeScript or linting errors
- Only show the dropzone when the user drags into it rather than when entering the document. This will prevent multiple announcements when we add feedback for screenreaders, in case there's multiple `FileUpload`s on the page - Add a test to check if the user is dragging files before showing dropzone - Fix disappearance of the dropzone due to many `dragleave` events being triggered as user drags over the different elements inside the wrapper - Separate the handler of `drop` event as it doesn't need the same complexity as the `dragleave` one before hiding the dropzone. The component still relies on the native `<input>` receiving the files being dropped, as it ensures a `change` event gets triggered on drop (which we'd have to simulate if setting its `files` properties programmatically).
It happened when dragging from the button to the span or the opposite, because Safari does not fill the `relatedTarget` on `dragleave`, making our code believe user had left the window. To accomodate for that, we use `dragenter` to also hide the drop zone when entering an element that's not the wrapper. This may still leave a gap where the component is at the edge of the viewport, either because of scrolling or in a iframe. Will explore in next commit.
Announces when users enter or leave the drop zone. Note: this seems to only be announced by Voice Over when Safari is in the foreground, even when using `aria-live="assertive"`
Button replaces entire native file upload. This replacement has a within it a "psuedo button" span that has the same focus, hover and active behaviour as a secondary button.
Hide the button text entirely from screen reader and add button content as `aria-label`. This should lead to more consistent reading out of the button content.
- Tests now use the correct selector for when the input has been visually replaced by the button. - Added tests for button `aria-label`. - Added test for clicking on different elements within button
Adjustments made to the the file upload so that in the event of an error the correct part of the page can be linked to.
Use button element for entire input replacement
This ensure hints and errors are associated with the `<button>` for assistive technology users.
The option also controls the rendering of the translation strings, as they're not needed if the component does not run JavaScript enhancements. In the unlikely case that someone would want the translation strings, but not run our JavaScript enhancements, we could add a new option. In the meantime, the `attributes` option allows them to manually add the data attributes.
As JavaScript enhancements are now optional, the examples loaded on the page needed to be `javascript`. Otherwise the `render` helper would throw as it'd try to initialise the component on `null`, as `document.querySelectorAll('govuk-file-upload')` wouldn't find any element.
The `render` helper assumed all examples of a component for which a JavaScript component exist would have a `data-module` and as such forced initialising a JavaScript component when no element with the appropriate `data-module` was found on the page. This behaviour is handy to spot if a component suddenly stops rendering `data-module`, so we don't want to change the `render` function. For the File Upload, we can decide whether the error is relevant in the test, based on whether the `javascript` macro option is set.
The `default` example is now back to rendering the native component, so we'll want a test for the JavaScript enhanced version
As part of post-audit investigations, a spike into layering a new UI onto the File upload component that should be a bit more friendly to assistive technologies. alphagov/govuk-design-system#4031
Note: This hasn't actually been tested across browsers and ATs yet,
Changes
multiple
,accepts
andcapture
attributes; etc.)Thoughts