Skip to content

Update downloads content without full page reload #1312

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 17, 2025

Conversation

saundefined
Copy link
Member

No description provided.

Copy link
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

Copy link
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
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
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
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.

@derickr
Copy link
Member

derickr commented Aug 15, 2025

Will this all still work if javascript is disabled too?

@cmb69
Copy link
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).

Co-authored-by: Shivam Mathur <[email protected]>
@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
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