Skip to content
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

Fix ingredient form field DOM ids #2167

Merged
merged 5 commits into from
Aug 10, 2021

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Aug 10, 2021

What is this pull request for?

Before the IDs where not unique; producing duplicated DOM ids for multiple instances of the same element type on the same page.

Since we want to be able to access the input via JS we need to have a unique DOM ID.

Also removes a lot of superfluous DOM ids that Rails generates to reduce DOM warnings.

Checklist

Before the ID was not unique producing duplicated DOM ids for multiple instances
of the same element type on the same page.

Since we want to be able to access the input via JS
we need to have a unique DOM ID.
@tvdeyen tvdeyen force-pushed the uniq-ingredient-form_field_id branch from 8018dff to e096660 Compare August 10, 2021 09:27
@tvdeyen tvdeyen changed the title Uniq ingredient form field Fix ingredient form field DOM ids Aug 10, 2021
These editors need a unique dom id to work correctly
with their js events.
These ingredients do not have JS events and the id
rails generates is not unique, so we remove them to avoid
DOM exceptions/warnings.
Make the input field readonly and remove the hidden field.
That way we can still see the link value.
@tvdeyen tvdeyen force-pushed the uniq-ingredient-form_field_id branch from e096660 to 0a4b407 Compare August 10, 2021 09:34
@tvdeyen tvdeyen marked this pull request as ready for review August 10, 2021 09:35
@tvdeyen tvdeyen requested a review from a team August 10, 2021 09:36
@tvdeyen tvdeyen added this to the 6.0 milestone Aug 10, 2021
Copy link
Contributor

@robinboening robinboening left a comment

Choose a reason for hiding this comment

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

Good catch and nice improvements! :shipit:

@tvdeyen tvdeyen merged commit 95c93b8 into AlchemyCMS:main Aug 10, 2021
@tvdeyen tvdeyen deleted the uniq-ingredient-form_field_id branch August 10, 2021 11:09
tvdeyen added a commit to tvdeyen/alchemy-solidus that referenced this pull request Aug 11, 2021
We need to have unique DOM ids for ingredient editors

Follows AlchemyCMS/alchemy_cms#2167
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.

2 participants