Skip to content

Commit e5b32c1

Browse files
sethherrclaude
andcommitted
Upload the register photo through UI::Forms::FileUpload
The register flow had its own file input, buttons and controller, which is what the shared component already does - minus the direct upload, so that moves into the component behind a direct_upload: flag: nameless field, hidden signed id, and the upload in ui--forms--file-upload. Off by default, so the org avatar, parking notification and registration sequence fields are untouched. Register::Photo's controller and its four translations go with it. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b67ae03 commit e5b32c1

9 files changed

Lines changed: 75 additions & 119 deletions

File tree

app/components/register/step2/component.en.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,8 @@ en:
7070
stolen: Stolen
7171
stolen_description: This %{cycle_type} has been stolen
7272
student_id: Student ID
73-
take_photo: Take Photo
7473
third_color_blank: Third color
7574
up_to_3: up to 3
76-
upload: Upload
77-
upload_failed: upload failed
78-
uploading: uploading…
7975
very_very_few_bikes_html: >-
8076
Very <em>very</em> few %{cycle_types} are made without serials. Unless this
8177
%{cycle_type}:

app/components/register/step2/component.html.erb

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -112,47 +112,14 @@
112112
</div>
113113

114114
<% if show_photo? %>
115-
<div
116-
class="tw:mb-4"
117-
data-controller="register--photo"
118-
data-register--photo-url-value="<%= rails_direct_uploads_path %>"
119-
data-register--photo-uploading-value="<%= translation(".uploading") %>"
120-
data-register--photo-failed-value="<%= translation(".upload_failed") %>"
121-
>
115+
<div class="tw:mb-4">
122116
<span class="twlabel">
123117
<%= translation(".attach_photo", cycle_type:) %>
124118
<%= optional_badge %>
125119
</span>
126120

127-
<%#
128-
Nameless (so the form doesn't go multipart for a file it never posts) - the photo
129-
goes browser -> bucket and the signed id below is what submits
130-
%>
131-
132-
<%= tag.input type: "file", id: "register_photo", accept: ImageUploader.permitted_extensions.join(","),
133-
class: "tw:hidden",
134-
data: {
135-
"register--photo-target": "input",
136-
action: "change->register--photo#changed"
137-
} %>
138-
139-
<%= hidden_field_tag :image_signed_id, nil, data: {"register--photo-target": "signedId"} %>
140-
141-
<div class="tw:mt-2 tw:grid tw:grid-cols-2 tw:gap-2">
142-
<%= render UI::Button::Component.new(text: translation(".take_photo"), color: :secondary,
143-
data: {action: "register--photo#takePhoto"}) %>
144-
145-
<%= render UI::Button::Component.new(text: translation(".upload"), color: :secondary,
146-
data: {action: "register--photo#upload"}) %>
147-
</div>
148-
149-
<p
150-
class="
151-
tw:mt-1.5 tw:truncate tw:text-xs tw:font-medium tw:text-gray-700
152-
tw:dark:text-gray-300
153-
"
154-
data-register--photo-target="filename"
155-
></p>
121+
<%= render UI::Forms::FileUpload::Component.new(form_builder: f, attribute: :image,
122+
accept: ImageUploader.permitted_extensions, direct_upload: true) %>
156123

157124
<p
158125
class="

app/components/ui/forms/file_upload/component.en.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,5 @@ en:
88
choose_or_drop_file: Click or drop to choose file
99
no_file_chosen: No file chosen
1010
take_picture: Take picture
11+
upload_failed: upload failed
12+
uploading: uploading…

app/components/ui/forms/file_upload/component.html.erb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
<div
33
data-controller="ui--forms--file-upload"
44
data-ui--forms--file-upload-placeholder-value="<%= @placeholder %>"
5+
<%# Blank unless direct_upload - the controller only uploads when it has somewhere to put it %>
6+
data-ui--forms--file-upload-url-value="<%= rails_direct_uploads_path if @direct_upload %>"
7+
data-ui--forms--file-upload-uploading-value="<%= translation(".uploading") %>"
8+
data-ui--forms--file-upload-failed-value="<%= translation(".upload_failed") %>"
59
data-action="drop->ui--forms--file-upload#drop dragover@document->ui--forms--file-upload#dragOver dragleave@document->ui--forms--file-upload#endDrag drop@document->ui--forms--file-upload#endDrag"
610
>
711
<%#
@@ -32,6 +36,10 @@
3236
<%# sr-only keeps the native input focusable and in the accessibility tree; the label below is the visible, clickable button. %>
3337
<%= @form_builder.file_field(@attribute, @html_options) %>
3438

39+
<% if @direct_upload %>
40+
<%= hidden_field_tag "#{@attribute}_signed_id", nil, data: {"ui--forms--file-upload-target": "signedId"} %>
41+
<% end %>
42+
3543
<%# Stimulus has no default event for a label, so name click explicitly. %>
3644
<%= @form_builder.label @attribute, label_content, class: @label_classes, data: {action: "click->ui--forms--file-upload#chooseFile"} %>
3745

app/components/ui/forms/file_upload/component.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class Component < ApplicationComponent
88
# otherwise center along with the button next to it.
99
LABEL_CLASSES = "tw:mb-0 tw:whitespace-nowrap tw:peer-focus-visible:ring-3 tw:peer-focus-visible:ring-blue-500/40"
1010

11-
def initialize(form_builder:, attribute:, accept: nil, camera: nil, html_options: {})
11+
def initialize(form_builder:, attribute:, accept: nil, camera: nil, direct_upload: false, html_options: {})
1212
@form_builder = form_builder
1313
@attribute = attribute
1414
@placeholder = translation(".no_file_chosen")
@@ -21,11 +21,15 @@ def initialize(form_builder:, attribute:, accept: nil, camera: nil, html_options
2121
@attachment_url = attached_url
2222
@thumbnail_url = thumbnail_version_url || @attachment_url
2323

24+
# The browser uploads to storage itself and the form carries the blob's signed id,
25+
# so the field is nameless - it must not also post the bytes
26+
@direct_upload = direct_upload
2427
@html_options = {
2528
class: "tw:peer tw:sr-only",
2629
accept: accept_list.join(",").presence,
2730
data: {"ui--forms--file-upload-target": "input", action: "ui--forms--file-upload#display"}
2831
}.merge(html_options)
32+
@html_options[:name] = nil if direct_upload
2933

3034
# Style the label as a UI::Button; the focus ring is driven by the peer (sr-only) input.
3135
@label_classes = UI::Button::Component.build_classes(color: :secondary, size: :md, html_class: LABEL_CLASSES)

app/javascript/controllers/register/photo_controller.js

Lines changed: 0 additions & 72 deletions
This file was deleted.

app/javascript/controllers/ui/forms/file_upload_controller.js

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,25 @@
11
import { Controller } from '@hotwired/stimulus'
2+
import { DirectUpload } from '@rails/activestorage'
23

34
// Connects to data-controller='ui--forms--file-upload'
45
// Shows the selected filename (or a count for multiple files) in the field, and
56
// frames the controls as a drop target while a file is dragged over the page.
67
export default class extends Controller {
7-
static targets = ['input', 'filename', 'dropZone']
8-
static values = { placeholder: String }
8+
static targets = ['input', 'filename', 'dropZone', 'signedId']
9+
static values = { placeholder: String, url: String, uploading: String, failed: String }
10+
11+
connect () {
12+
this.boundHold = this.hold.bind(this)
13+
this.form?.addEventListener('submit', this.boundHold)
14+
}
15+
16+
disconnect () {
17+
this.form?.removeEventListener('submit', this.boundHold)
18+
}
19+
20+
get form () {
21+
return this.element.closest('form')
22+
}
923

1024
// Both buttons open the one input; `capture` is what sends it to the camera.
1125
takePicture () {
@@ -69,6 +83,43 @@ export default class extends Controller {
6983
files.length === 0
7084
? this.placeholderValue
7185
: files.length === 1 ? files[0].name : `${files.length} files`
86+
if (this.urlValue) this.upload(files[0])
87+
}
88+
89+
// Only when the field is nameless (direct_upload) - the form then carries the blob's
90+
// signed id rather than the bytes.
91+
upload (file) {
92+
if (!file) return
93+
94+
this.xhr?.abort() // Picking again shouldn't leave the discarded file uploading
95+
this.signedIdTarget.value = ''
96+
this.filenameTarget.textContent = `${file.name}${this.uploadingValue}`
97+
98+
const upload = new DirectUpload(file, this.urlValue, this)
99+
this.currentUpload = upload
100+
this.pending = new Promise((resolve) => upload.create((error, blob) => {
101+
resolve()
102+
if (this.currentUpload !== upload) return // A newer pick owns the field now
103+
104+
this.pending = null
105+
if (!error) this.signedIdTarget.value = blob.signed_id
106+
this.filenameTarget.textContent = error ? `${file.name}${this.failedValue}` : file.name
107+
}))
108+
}
109+
110+
// DirectUpload delegate hook - the handle that makes a discarded upload cancellable
111+
directUploadWillStoreFileWithXHR (xhr) {
112+
this.xhr = xhr
113+
}
114+
115+
// Submitting mid-upload would drop the file, so hold the form until the blob lands.
116+
// A failed upload submits anyway - the rest of the form matters more.
117+
async hold (event) {
118+
if (!this.pending) return
119+
120+
event.preventDefault()
121+
await this.pending
122+
this.form.requestSubmit()
72123
}
73124
}
74125

spec/integration/registration/register_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ def start_registration
170170
it "PUTs the photo to the bucket and serves it from the storage domain" do
171171
start_registration
172172

173-
attach_file("register_photo", image_path, make_visible: true)
173+
attach_file("bike_image", image_path, make_visible: true)
174174
expect(page).to have_content("bike_photo-landscape.jpeg")
175175
expect(page).to have_no_content("uploading", wait: 20) # A real cross-origin PUT
176176

spec/requests/register_request_spec.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,13 +353,13 @@ def phone_statuses_watched
353353
# An address nothing has proven yet, so the confirmation is still pending - and the
354354
# upload goes to a signed-in-only endpoint, so it isn't offered either
355355
expect(response.body).to include "confirmation link to your email"
356-
expect(response.body).to_not include "register_photo"
356+
expect(response.body).to_not include "bike_image"
357357

358358
# Confirming clears the alert, but the upload still needs an account
359359
b_param.confirm_email!
360360
get register_path(b_param_token: b_param.id_token, step: 2)
361361
expect(response.body).to_not include "confirmation link to your email"
362-
expect(response.body).to_not include "register_photo"
362+
expect(response.body).to_not include "bike_image"
363363
end
364364

365365
it "hides the phone field, showing it for the statuses bikes/new does" do
@@ -443,7 +443,7 @@ def phone_statuses_watched
443443
get register_path(b_param_token: b_param.id_token, step: 2)
444444
expect(response.status).to eq 200
445445
expect(response.body).to_not include "confirmation link to your email"
446-
expect(response.body).to include "register_photo"
446+
expect(response.body).to include "bike_image"
447447
end
448448
end
449449

0 commit comments

Comments
 (0)