Fix removing existing image and adding a new one #4599
Merged
Conversation
This file contains 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
This way we save a couple of database insertions and browser requests.
We introduced a bug in commit acbd1b0. When editing a record and removing an existing image, we don't remove the HTML fields associated with that image but simply hide them, and then we add fields to create a new image when clicking on "Add image". This is standard cocoon behavior. However, since in the case of images there's a `has_one` relation, cocoon doesn't add unique identifiers to the new fields, generating duplicate IDs, which is invalid HTML. Since there's a duplicate file input ID, clicking on the "Choose image" label we aren't clicking on the new input but on the old one. This means we aren't correctly attaching an image. The tests passed because Capybara uses the equivalent of a keyboard to select the field, and in this case everything worked properly. So we need to delete the existing elements before inserting new ones. We're adding a test to check there aren't duplicate IDs.
javierm
force-pushed
the
invalid_image_fields_html
branch
from
6ea3e98
to
c0a6bf5
Compare
In commit cc6f939 we made the images and documents file inputs invisible (instead of using `display: none`) in order to make it possible to attach images and documents using the keyboard. However, since the error messages associated to these inputs has the same HTML class as the inputs, we were also hiding them (the `display: none` didn't affect the error messages because they've also got the `is-visible` class). Using the `[type=file]` selector we make it more explicit that we only want to style these inputs. I'm not adding a test for this scenario because technically the text is there and I'm not sure how to test for the presence of invisible elements.
taitus
approved these changes
Jul 30, 2021
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.
References
Objectives