fix: reply storage is updated with the new content on file upload

[aegroto]

Description

Images and other file uploads update the draft reply, fixes #1522

Screenshots

Unfortunately it's difficult to show the difference using screenshots, maybe I can try with a screen recording later?

Additional Context

Nothing to report.

Checklist

Are your changes backwards compatible? Please answer below: Yes

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below: 8. Throughly tested manually on dummy posts, no automatic tests.

For frontend changes: Tested on mobile? Please answer below: Yes, they work.

Did you introduce any new environment variables? If so, call them out explicitly here: No

[ekzyis]

Hey, thanks for the PR! Videos are nice, especially for UI/UX stuff but no need here anymore, I saw that this fixes it:

2024-11-12.21-43-17.mp4

This is a nice way to fix it as simple as possible but ideally, I'd like to see a fix that triggers a change event so we can reuse the logic in this function:

stacker.news/components/form.js

Lines 489 to 499 in d1c770d

	const onChangeInner = useCallback((e) => {
	field?.onChange(e)
	if (storageKey) {
	window.localStorage.setItem(storageKey, e.target.value)
	}
	if (onChange) {
	onChange(formik, e)
	}
	}, [field?.onChange, storageKey, onChange])

I tried the following when #1522 was created but for some reason, the callback doesn't run (I just tried again).

diff --git a/components/form.js b/components/form.js
index a67853f8..c38d6142 100644
--- a/components/form.js
+++ b/components/form.js
@@ -370,6 +370,7 @@ export function MarkdownInput ({ label, topLevel, groupClassName, onChange, onKe
                 const s3Keys = [...text.matchAll(AWS_S3_URL_REGEXP)].map(m => Number(m[1]))
                 updateUploadFees({ variables: { s3Keys } })
                 setSubmitDisabled?.(false)
+                innerRef.current.dispatchEvent(new Event('input', { bubbles: true, value: text }))
               }}
               onError={({ name }) => {
                 let text = innerRef.current.value

Can you check if you can find out how to get this to work or at least why it doesn't work? It would be a cleaner solution because it reuses code and makes changing the input via image upload consistent with any other change since there's more code in that callback than just window.localStorage.setItem.

[aegroto]

Hello, thanks for the detailed review! I acknowledge that a solution calling the an event would be better. Just wondering, may moving the whole logic the local storage from MarkdownInput to the parent component, in the onChange event, work for you?

[aegroto]

I have converted the code to work with events by following this snippet: facebook/react#10135 (comment)

The issue may be related to the fact that change events are dropped if the text is not changed in practice, therefore the setNativeValue function uses some sort of reflection to bypass this mechanism.

I admit this looks kind of hacky to me and would prefer a React-based solution instead, but if you'd like to work with events this is a deal.

[ekzyis]

Wow, awesome that you found out why it wasn't working!

Added some suggestions how this could be even improved.

Requesting changes because the Formik handler should definitely be called imo.

[aegroto]

Thanks for your feedback, I have integrated your changes and I confirm everything is still working as expected.