Fix attaching files using the keyboard

We were hiding the file input and styling the label as a button instead.
Since clicking on a label has the same effect as clicking on the input,
the input worked properly for mouse and touch screen users.

However, hiding the input makes it inaccessible for keyboard users,
since labels don't get keyboard focus, but inputs do.

So we must not hide the input but make it invisible instead. But we
still need to hide the input (alongside the label) after a file has been
attached.

We could add some extra JavaScript to hide the input when we hide the
label. Since the JavaScript is already quite complex and my first few
attempts at changing it failed, I've opted to assume that the input (and
its label) must be hidden whenever there's already a file name, and
implement that rule with CSS.

Note we're using the `:focus-within` pseudoclass to style a label when
focus is on the input. This rule (at the time of writing) is only
supported by 93.5% of the browsers. Keyboard users without a screen
reader and using the other 6.5% of the browsers will still be able to
focus on the field but might not notice the field has received focus.
Since the percentage of affected users will decrease over time and until
now 100% of keyboard users were completely unable to focus on these
fields, for now we think this is a good-enough solution.

app/assets/javascripts/documentable.js

@@ -45,7 +45,6 @@
           App.Documentable.setInputErrors(data);
           $(data.destroyAttachmentLinkContainer).find("a.delete:not(.remove-nested)").remove();
           $(data.addAttachmentLabel).addClass("error");
-          $(data.addAttachmentLabel).show();
         },
         done: function(e, data) {
           var destroyAttachmentLink;
@@ -54,7 +53,6 @@
           App.Documentable.setProgressBar(data, "complete");
           App.Documentable.setFilename(data, data.result.filename);
           App.Documentable.clearInputErrors(data);
-          $(data.addAttachmentLabel).hide();
           destroyAttachmentLink = $(data.result.destroy_link);
           $(data.destroyAttachmentLinkContainer).html(destroyAttachmentLink);
           if (input.lockUpload) {

app/assets/javascripts/imageable.js

@@ -43,7 +43,6 @@
           App.Imageable.clearPreview(data);
           $(data.destroyAttachmentLinkContainer).find("a.delete:not(.remove-nested)").remove();
           $(data.addAttachmentLabel).addClass("error");
-          $(data.addAttachmentLabel).show();
         },
         done: function(e, data) {
           var destroyAttachmentLink;
@@ -52,7 +51,6 @@
           App.Imageable.setProgressBar(data, "complete");
           App.Imageable.setFilename(data, data.result.filename);
           App.Imageable.clearInputErrors(data);
-          $(data.addAttachmentLabel).hide();
           App.Imageable.setPreview(data);
           destroyAttachmentLink = $(data.result.destroy_link);
           $(data.destroyAttachmentLinkContainer).html(destroyAttachmentLink);

app/assets/stylesheets/mixins/uploads.scss

@@ -20,13 +20,17 @@
       p {
         margin-bottom: 0;
       }
+
+      &:focus-within label {
+        outline: $outline-focus;
+      }
     }
 
     .attachment-errors {
 
       > .js-image-attachment,
       > .js-document-attachment {
-        display: none;
+        @include element-invisible;
 
         ~ .error {
           display: inline-block;
@@ -47,6 +51,11 @@
     &:empty {
       display: none;
     }
+
+    &:not(:empty) + .document-attachment,
+    &:not(:empty) + .image-attachment {
+      display: none;
+    }
   }
 
   .loading-bar {

app/components/documents/fields_component.html.erb

@@ -8,12 +8,12 @@
   </div>
 
   <div class="small-12 column attachment-actions">
+    <p class="file-name small-9 column"><%= file_name %></p>
+
     <div class="small-9 column action-add attachment-errors document-attachment">
       <%= file_field %>
     </div>
 
-    <p class="file-name small-9 column"><%= file_name %></p>
-
     <div class="small-3 column action-remove text-right">
       <%= destroy_link %>
     </div>

app/components/images/fields_component.html.erb

@@ -10,12 +10,12 @@
   <%= render_image(image, :thumb, false) if image.attachment.exists? %>
 
   <div class="small-12 column attachment-actions">
+    <p class="file-name small-9 column"><%= file_name %></p>
+
     <div class="small-9 column action-add attachment-errors image-attachment">
       <%= file_field %>
     </div>
 
-    <p class="file-name small-9 column"><%= file_name %></p>
-
     <div class="small-3 column action-remove text-right">
       <%= destroy_link %>
     </div>

spec/shared/system/nested_documentable.rb

@@ -77,12 +77,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
 
       click_link "Add new document"
       within "#nested-documents" do
-        document = find(".document input[type=file]", visible: :hidden)
-        attach_file(
-          document[:id],
-          Rails.root.join("spec/fixtures/files/empty.pdf"),
-          make_visible: true
-        )
+        attach_file "Choose document", Rails.root.join("spec/fixtures/files/empty.pdf")
 
         expect(page).to have_css ".loading-bar.complete"
       end
@@ -109,12 +104,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na
       within "#nested-documents" do
         input = find("input[name$='[title]']")
         fill_in input[:id], with: "My Title"
-        document_input = find("input[type=file]", visible: :hidden)
-        attach_file(
-          document_input[:id],
-          Rails.root.join("spec/fixtures/files/empty.pdf"),
-          make_visible: true
-        )
+        attach_file "Choose document", Rails.root.join("spec/fixtures/files/empty.pdf")
 
         expect(page).to have_css ".loading-bar.complete"
       end
@@ -321,8 +311,7 @@ def documentable_attach_new_file(path, success = true)
   click_link "Add new document"
 
   document = all(".document").last
-  document_input = document.find("input[type=file]", visible: :hidden)
-  attach_file(document_input[:id], path, make_visible: true)
+  attach_file "Choose document", path
 
   within document do
     if success

spec/shared/system/nested_imageable.rb

@@ -38,12 +38,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
       visit send(path, arguments)
 
       click_link "Add image"
-      image_input = find(".image").find("input[type=file]", visible: :hidden)
-      attach_file(
-        image_input[:id],
-        Rails.root.join("spec/fixtures/files/clippy.jpg"),
-        make_visible: true
-      )
+      attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg")
 
       expect(page).to have_selector ".file-name", text: "clippy.jpg"
     end
@@ -67,12 +62,7 @@ shared_examples "nested imageable" do |imageable_factory_name, path, imageable_p
       click_link "Add image"
       input_title = find(".image input[name$='[title]']")
       fill_in input_title[:id], with: "Title"
-      image_input = find(".image").find("input[type=file]", visible: :hidden)
-      attach_file(
-        image_input[:id],
-        Rails.root.join("spec/fixtures/files/clippy.jpg"),
-        make_visible: true
-      )
+      attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg")
 
       if has_many_images
         expect(find("input[id$='_title']").value).to eq "Title"
@@ -271,8 +261,7 @@ def imageable_attach_new_file(_imageable_factory_name, path, success = true)
   click_link "Add image"
   within "#nested-image" do
     image = find(".image")
-    image_input = image.find("input[type=file]", visible: :hidden)
-    attach_file(image_input[:id], path, make_visible: true)
+    attach_file "Choose image", path
     within image do
       if success
         expect(page).to have_css(".loading-bar.complete")

spec/system/admin/widgets/cards_spec.rb

@@ -252,11 +252,8 @@ describe "Cards", :admin do
 
   def attach_image_to_card
     click_link "Add image"
-    image_input = all(".image").last.find("input[type=file]", visible: false)
-    attach_file(
-      image_input[:id],
-      Rails.root.join("spec/fixtures/files/clippy.jpg"),
-      make_visible: true)
+    attach_file "Choose image", Rails.root.join("spec/fixtures/files/clippy.jpg")
+
     expect(page).to have_field("widget_card_image_attributes_title", with: "clippy.jpg")
   end
 end