-
Notifications
You must be signed in to change notification settings - Fork 20
Fixing ID randomization but keeping the stable selector #731
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
Conversation
Discussion can be followed from previous Closed PR: #729 |
Have tested and this works with the address lookup webform element still. Have noticed that with this fix the edit-actions doesn't get the collision aviodance random ID if it has a --3 etc already. But I think this can still cause a problem so I would suggest applying it too all containers that start with |
I think this looks good to me. I don't think we are using ids anywhere in our JS/CSS for styling (which in any case we couldn't since they would be changing each time we reload a page). @andybroomfield you've been giving this a good test. If you are okay with how it's working, then I'm happy for you t approve it. |
@markconroy I think its good, just the fix to handle use cases where the id starts with |
Discussed briefly at Merge Tuesday, sounds like it needs a little more work. @PardeepSinghBali are you OK to review ? |
Yes sure, I will |
done and changes pushed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @PardeepSinghBali this is working as expected, and the id randomisation is applied to all iterations of edit-actions.
Perfect! |
What does this change?
This PR addresses an issue related to form container ID randomization for improved accessibility and Address Dropdown not populating the addresses at drupal Form. Previously, the localgov_base_preprocess_container() function was appending a random suffix to the edit-actions form container ID. This introduced potential issues with JavaScript selectors that rely on the #edit-actions ID, as they were no longer able to target the element due to the dynamic nature of the ID.
This change ensures that:
The ID is randomized for accessibility, but we still maintain a stable selector (data-drupal-selector) to be used by JavaScript.
JavaScript selectors that depend on the original ID (edit-actions) will work as expected, avoiding potential breakage in behavior.
The random ID is applied to the id attribute, but the original ID is retained as data-drupal-selector for stable targeting.
How to test
Check the edit-actions form container element:
On the main branch, inspect the container and check the id attribute. It should be static (e.g., #edit-actions).
On this branch, inspect the same container and verify that the id has been randomized (e.g., #edit-actions--) but the data-drupal-selector remains as edit-actions.
How can we measure success?
No errors or broken JavaScript functionality related to the #edit-actions container ID after the change.
JavaScript selectors that use data-drupal-selector="edit-actions" work as expected.
Improved accessibility with randomized form container IDs while keeping the stable selector intact.
Have we considered potential risks?
Images
Accessibility