Fix project attribute text/integer fields not saving on blur - #24813
Open
tiago-lima-4linux wants to merge 1 commit into
Open
Fix project attribute text/integer fields not saving on blur#24813tiago-lima-4linux wants to merge 1 commit into
tiago-lima-4linux wants to merge 1 commit into
Conversation
TextInputComponent only wired keydown.esc, so text and integer format project attributes (IntegerInputComponent inherits from TextInputComponent) never submitted unless the user happened to press Enter, which only worked by accident via the browser's native single-input-form implicit submission, not through application JS. Clicking away to edit another field, or tabbing out, silently discarded the value with no error shown. DateInputComponent already wires keydown.enter and change to inplace-edit#submitForm. This applies the same bindings to TextInputComponent so it behaves consistently. The existing spec file's test titles already said "does not add a submit-on-change Stimulus action", copied from date_input_component_spec.rb, but the assertions were never updated to actually check for it, so the gap went uncaught. Updated the assertions to match.
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What's wrong
Project attributes (project-level custom fields) of format string or integer silently fail to save when editing them inline from the project overview page, unless the user happens to press Enter. Clicking away to edit another attribute, or tabbing out, discards the typed value with no error shown.
IntegerInputComponentinherits directly fromTextInputComponentwithout overriding the relevant method, so it has the exact same gap.Why
TextInputComponent#additional_argumentsonly wires:DateInputComponent, right next to it, correctly wires:TextInputComponentis missing both thekeydown.enterand, more importantly, thechangebinding. Pressing Enter appears to work today, but only by accident: the edit form contains a single visible text input, so the browser's native implicit-submission-on-Enter kicks in regardless of application JS. Any interaction other than Enter (blur, Tab, clicking another field) never sends a request at all.Interestingly,
text_input_component_spec.rbalready had a test titled "does not add a submit-on-change Stimulus action whe show_action_buttons is false" (same title, same typo, as the equivalent test indate_input_component_spec.rb), but its assertion only checked forkeydown.esc, never forchange->inplace-edit#submitForm. Looks like the spec was copied over when this component was written, but the matching implementation and assertion were not.What this changes
keydown.enter->inplace-edit#submitFormandchange->inplace-edit#submitFormbindings thatDateInputComponentalready has, toTextInputComponent#additional_arguments.text_input_component_spec.rbto actually assert the new bindings are present (and absent whenshow_action_buttons: false), matchingdate_input_component_spec.rb's coverage.Testing
PATCH .../updatefires, value persists after reload).devas of this writing (checked before opening this PR).date_input_component_spec.rb, and I'd appreciate a check from CI on that.