Change form actions to fetch for submit review box

[HesterG]

Co-author: @wxiaoguang

Close #25096

The way to fix it in this PR is to change form submit to fetch using formData, and add flags to avoid post repeatedly.
Should be able to apply to more forms that have the same issue after this PR.

After:

Untitled.mov

Update: screenshots from /devtest

[wxiaoguang]

It looks like a specialized patch for the "review" form.

I think it should be a general approach: if a form submit button is marked as using "fetch", then submit the form by fetch, instead of writing magic code like "data-type" or manually constructing the request body.

[HesterG]

It looks like a specialized patch for the "review" form.

I think it should be a general approach: if a form submit button is marked as using "fetch", then submit the form by fetch, instead of writing magic code like "data-type" or manually constructing the request body.

Then maybe revert back to form submit but add a submit event listener? I am not sure how to make a POST fetch a general approach because every post may need different data as body

[wxiaoguang]

var data = new FormData(theFormElement)

[HesterG]

var data = new FormData(theFormElement)

I will check this, thanks.

[wxiaoguang]

Almost done from my side. There are many details. Here are the screenshots

---

---

[HesterG]

Updated description.

[delvh]

I don't understand this code.
First, you declare these three parameters, but then you delete them again?
And below, you execute them if present?

[wxiaoguang]

Otherwise, the ...opts, below would overwrite the createTippy's onXxx

[silverwind]

Mutating function args is bad, please avoid in the future. In this case, you could just create a new object with object destructuring syntax:

const {onHide, onShow, onDestroy, ...other} = opts;

Now other holds all opts besides those three and does not mutate the args.

[wxiaoguang]

I do not see any problem for doing this in JS. Because in JS nothing no object is really immutable, the caller should create new objects for each call.

And, introducing other makes the code unclear for when to use opts or when to use other.

But if you prefer, I can take it in #25253 by the way.

[silverwind]

Yes, the caller should, but in case you have no control over the caller, it's better to not mutate to not intruduce bugs in the caller. The only safe mutations are for primitive types like string, but I'd argue for clarity it's better to not even mutate those.

[silverwind]

We might consider enabling https://eslint.org/docs/latest/rules/no-param-reassign for this.

[silverwind]

This was definitely merged too fast.

[wxiaoguang]

This was definitely merged too fast.

Yup, sometimes too fast, sometimes too slow.