Skip to content

Update downloads content without full page reload#1312

Merged
saundefined merged 6 commits intophp:masterfrom
saundefined:downloads-ajax
Aug 17, 2025
Merged

Update downloads content without full page reload#1312
saundefined merged 6 commits intophp:masterfrom
saundefined:downloads-ajax

Conversation

@saundefined
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 13, 2025

🚀 Regression report for commit 0a43db5 is at https://web-php-regression-report-pr-1312.preview.thephp.foundation

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 13, 2025

🚀 Preview for commit 0a43db5 can be found at https://web-php-pr-1312.preview.thephp.foundation

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 13, 2025

If there are two changes, there are two asynchronous requests. If the first response arrives after the second, the instructions would not match the latest user selection. I'm not sure how to solve that. A mitigation could be to delay the request for some fraction of a second (what might be a good idea anyway).

Also, error handling should be improved; logging to the console won't usually be seen by the user, but worse, if the server reponds with an error status, the script would try to go on. There should be a check for response.ok in addition to the .catch().

@saundefined
Copy link
Copy Markdown
Member Author

@cmb69 thanks!

If there are two changes, there are two asynchronous requests. If the first response arrives after the second, the instructions would not match the latest user selection. I'm not sure how to solve that. A mitigation could be to delay the request for some fraction of a second (what might be a good idea anyway).

What do you think about AbortController? Just tested locally, it solves this problem.
I'll send a commit with it now, if that option doesn't work, I'll roll it back.

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 13, 2025

What do you think about AbortController?

Interesting! I wasn't aware of that class, but it looks like a sensible way to solve the issue.

Comment thread downloads.php Outdated
@derickr
Copy link
Copy Markdown
Member

derickr commented Aug 15, 2025

Will this all still work if javascript is disabled too?

@cmb69
Copy link
Copy Markdown
Member

cmb69 commented Aug 15, 2025

If JS is not supported, it's just a normal form with an "Update Instructions" button. It might be better, though, to actually remove the button when JS is available (i.e. the concrete JS is supported by the browser), instead of relying on <noscript>, since JS might not be available even when supported by the browser (e.g. some old browsers may not support fetch(), and in that case the user can't use the form).

Comment thread downloads.php Outdated
Co-authored-by: Shivam Mathur <shivam_jpr@hotmail.com>
@saundefined saundefined merged commit b8d9b90 into php:master Aug 17, 2025
5 checks passed
@saundefined saundefined deleted the downloads-ajax branch August 17, 2025 14:09
marcosmarcolin pushed a commit to marcosmarcolin/web-php that referenced this pull request Oct 9, 2025
Co-authored-by: Shivam Mathur <shivam_jpr@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants