Block scene/performer submits with pending URLs#1123
Conversation
Gykes
left a comment
Gykes
left a comment
There was a problem hiding this comment.
For some reason I have lost the ability to wrap text in blocks. I beleive my key is broken.... sorry.
Just a quick once over. Didn't dig too deep into it.
| onChange={(e) => { | ||
| const value = e.currentTarget.value; | ||
| setNewURL(value); | ||
| onPendingURLChange?.(value.trim().length > 0); | ||
| }} |
There was a problem hiding this comment.
If im reading this correctly the onPendingURLChange notification is wired into onChange. It looks like the existing onPaste handler also calls setNewURL directly if it matches a site. I think if a user pastes in a URL, doesnt add, then saves it will get missed.
| pendingURLError={errors.pendingUrl?.message} | ||
| onPendingURLChange={(hasPendingURL) => | ||
| setValue("pendingUrl", hasPendingURL ? "pending" : "", { |
There was a problem hiding this comment.
Soring pending vs "" to drive a boolean check seems weird to me and potentially a smell down the line. I would make pendingURL a yup.boolean() or store the pending url.
There was a problem hiding this comment.
Seems like this is also the same on SceneForm.tsx and schema.ts
| pendingURLError={errors.pendingUrl?.message} | ||
| onPendingURLChange={(hasPendingURL) => | ||
| setValue("pendingUrl", hasPendingURL ? "pending" : "", { | ||
| shouldValidate: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Seems like this is duplicated. My genera rule of thumb is 2+ times to break it out into a hook or a shared component to reduce LOC
2 participants
When I started contributing to stashdb, I constantly forgot to click the "Add" button when adding a URL to a scene draft.
This PR adds checks to the scene and performer draft forms to ensure that the user actually clicks Add after entering a URL.
Screenshot:
Drafted with Codex, but I have looked through the code myself and it looks reasonable to me (although my React knowledge is limited)