Skip to content

Fix back button after form submit #2066

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix back button after form submit #2066

wants to merge 3 commits into from

Conversation

amitaibu
Copy link
Collaborator

@amitaibu amitaibu commented May 5, 2025

fixes #2063

  • Remove initBack which seems no longer used (?)
  • Modernize initDisableButtonsOnSubmit to use a better way to prevent double binding

@unhammer could you try this PR locally to confirm it works for you?

@unhammer
Copy link
Collaborator

unhammer commented May 5, 2025

AFAICT this change has no effect on the issue.

From how I read the code, initBack only does something if you've manually added a <button class="js-back">. I can't find this js-back class documented anywhere (git log --stat -S js-back only shows changes to helpers.js) and I can find no open source usages on github https://github.com/search?q=js-back+hsx+language%3Ahaskell&type=code so it seems ripe for deprecation, but I also don't see how it can be related to my issue.

I tried adding some logging to see whether initDisableButtonsOnSubmit was being run twice, it doesn't seem like it was being run twice before either. In any case, the issue remains on my end.

Did the issue disappear in your testing with this patch?

@amitaibu
Copy link
Collaborator Author

amitaibu commented May 5, 2025

Sorry, I thought I had fixed it, but re-checking it didn't. Let me try again.

@amitaibu amitaibu marked this pull request as draft May 5, 2025 16:09
@amitaibu
Copy link
Collaborator Author

amitaibu commented May 5, 2025

(My assumption is that after the form redirect, the new page isn't registered on the history)

@amitaibu
Copy link
Collaborator Author

amitaibu commented May 5, 2025

With Bug

with-bug-Peek.2025-05-05.19-27.mp4

With PR

Peek.2025-05-05.19-26.mp4

@unhammer
Copy link
Collaborator

unhammer commented May 5, 2025

OMG this seems to actually work :-D

I have to test it more extensively, but it seems like this does the trick.

I wonder what the reason was for replaceState instead of pushState there in the first place – could there be some difference between submitting and staying in the same url (except query parameters?) vs submitting and redirecting to a new url?

@amitaibu
Copy link
Collaborator Author

amitaibu commented May 6, 2025

I wonder what the reason was for replaceState instead of pushState there in the first place

Hopefully @mpscholten will remember 😄

@amitaibu amitaibu marked this pull request as ready for review May 6, 2025 04:54
@amitaibu amitaibu requested a review from mpscholten May 6, 2025 04:55
@@ -211,11 +186,11 @@ window.submitForm = function (form, possibleClickedButton) {
);
transitionToNewPage(request.response);
Turbolinks.clearCache();
if (urlPathnameWithQuery !== formAction) {

if (urlPathnameWithQuery !== formAction || urlPathnameWithQuery !== responseUrlPath) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for the back button

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Back button does not change contents after form submission + link click
2 participants