3689 check double clicks on submit #5317
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o_submits_with properties to the link/button to avoid double clicks
… consistent with other buttons
…led on the majority of controllers
…repeated requests
awwaiid
reviewed
Aug 17, 2025
| @@ -0,0 +1,24 @@ | |||
| # Simulates the user double clicking on an element specified by a css_selector | |||
| # by using the API provided by Ferrum to manipulate the browser directly. | |||
Collaborator
There was a problem hiding this comment.
"We tried other techniques (built in double-click method, for example), but this is the way we were able to reliable cause double button press bugs to be triggered (and thus validate when they are fixed)"
awwaiid
approved these changes
Aug 17, 2025
Collaborator
awwaiid
left a comment
awwaiid
left a comment
There was a problem hiding this comment.
This looks good!
We walked through it all today and to clarify -- this improves the user-experience but we haven't found any actual double-submits yet. This makes the UX for users more obvious.
Contributor
|
@Benjamin-Couey: Your PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes progress towards resolving #3689
Description
This PR modifies the helpers in
app/helpers/ui_helper.rbthat create buttons and links to always include thedisable_withproperty. This property will, upon the first click, disable the button/link and change its text to a configurable value ("Please wait.." by default) to prevent the user from making multiple redundant requests when double clicking a button that modifies the database.This should solve a large subset of issues with double clicks, though in my opinion there is further work that should be done.
Beyond what this PR addresses, there are a number of places where the site has links which make non-GET requests to the server. Ruby on Rails recommends against this and instead recommends using forms with submit buttons (such as those created by the
button_toAction View URL Helper). Relevant to the issue this would guard against double clicks. In the future, I will work on another PR to refactor ui_helper and the broader site to replace these non-GET links with form.In the discussion on the issue, the
turbo_submits_withproperty was proposed as an alternative todisable_with. However,turbo_submits_withonly works for buttons which submit a form and as mentioned above there are many cases where forms aren't used.Additionally, Turbo is currently only enabled for a very small subset of controller actions (currently just the new and show actions of the
app/controllers/distributionscontroller.rb) and enabling Turbo site-wide causes a number of tests to start failing. Diagnosing all of those tests and, if possible, reworking the site to work with Turbo enabled seemed outside the scope of this issue and so was set aside for now.Type of change
How Has This Been Tested?
I manually tested that double clicking on various buttons disabled them after the first click.
Automated tests were added for two pages where double clicks produced very noticeably unusual behavior to verify the addition of
disable_with.