Skip to content
This repository was archived by the owner on May 22, 2021. It is now read-only.

Make upload button focusable (accessibility/tab navigation)#574

Merged
ericawright merged 1 commit intomozilla:masterfrom
ehuggett:tab-upload
Oct 3, 2017
Merged

Make upload button focusable (accessibility/tab navigation)#574
ericawright merged 1 commit intomozilla:masterfrom
ehuggett:tab-upload

Conversation

@ehuggett
Copy link
Copy Markdown
Contributor

fixes #563

Copy link
Copy Markdown
Contributor

@ericawright ericawright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at this on Firefox on Mac.
Oddly, once in a while the #browse element will show as selected, and others it simply won't. You can see this often by uploading one file then clicking upload another. I can't seem to figure out why though....perhaps it is in the wrong file?
Nit: if you could change the double quotes to single in main.css that would be nice.
Thanks for this, and sorry I took so long to review.

assets/main.css Outdated
input[type='file'] {
display: none;
width: 0.1px;
height: 0.1px;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the width and height rules here don't seem necessary since it's display: absolute and opacity: 0

Copy link
Copy Markdown
Contributor Author

@ehuggett ehuggett Oct 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem important to me*, but just in case, those rules were responsible for the 'hidden' form input fitting behind the label used as a button

browse_size

edit: *the consequences of removing the size rules does not seem important, but just in case

@ehuggett
Copy link
Copy Markdown
Contributor Author

ehuggett commented Oct 1, 2017

Oddly, once in a while the #browse element will show as selected, and others it simply won't. You can see this often by uploading one file then clicking upload another. I can't seem to figure out why though....perhaps it is in the wrong file?

I can reproduce this with Firefox ESR on Debian, it never works after clicking upload another file. I think this is because the javascript that adds the focus and blur events only runs on DOMContentLoaded.

'uploadPageSizeMessage'
)}</em></span>
<form method="post" action="upload" enctype="multipart/form-data">
<input id="file-upload" type="file" name="fileUploaded" onchange=${upload} onfocus=${onfocus} onblur=${onblur} />
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The html attributes onfocus and onblur have the same issues as the css in Firefox, but it seems I can use them in the template as they get rewritten into javascript event listeners? (which do work in Firefox). I clearly need to read some documentation to understand this better, will it always remove & replace the html onfocus attribute? (becuase if it doesn't, this will brake).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be working for me now. 👍
Can you re-base so this can merge smoothly?

@ehuggett
Copy link
Copy Markdown
Contributor Author

ehuggett commented Oct 2, 2017

I think I just misused git, sorry.

I rebased, squashed and then force pushed so the reviewed/approved commits are no longer visible on github.

I think i can put this back the way it was before (and then only rebase on master), should i?

@ericawright
Copy link
Copy Markdown
Contributor

ericawright commented Oct 2, 2017

Actually, it looks correct now. I'll just test it again....

I see what you're worried about, the commit history was lost. That's not a problem at all :D

@ehuggett
Copy link
Copy Markdown
Contributor Author

ehuggett commented Oct 2, 2017

I've got the original branch back (and rebased it, without squashing). I should be able to force push that if you haven't (re)started yet?

@ericawright
Copy link
Copy Markdown
Contributor

Nope, this is perfectly good as is. I prefer having a smaller commit history anyway.

@ericawright ericawright merged commit d10ceac into mozilla:master Oct 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot access "Select file to upload button" using keyboard

2 participants