8000 Fixing ID randomization but keeping the stable selector by PardeepSinghBali · Pull Request #731 · localgovdrupal/localgov_base · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 3 commits into from
Mar 27, 2025

Conversation

PardeepSinghBali
Copy link
Contributor

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

@PardeepSinghBali
Copy link
Contributor Author

Discussion can be followed from previous Closed PR: #729

@andybroomfield
Copy link
Contributor
andybroomfield commented Mar 19, 2025

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 edit-actions.
See the original issue #324 and PR #325.

Screenshot 2025-03-19 at 2 41 46 pm

Screenshot 2025-03-19 at 2 52 34 pm

@markconroy
Copy link
Member

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.

@andybroomfield
Copy link
Contributor

@markconroy I think its good, just the fix to handle use cases where the id starts with edit-actions as the original issue is the ID collision could still happen in those cases when the block is cached. /cc @PardeepSinghBali

@finnlewis
Copy link
Member

Discussed briefly at Merge Tuesday, sounds like it needs a little more work.

@PardeepSinghBali are you OK to review ?

@PardeepSinghBali
Copy link
Contributor Author

Discussed briefly at Merge Tuesday, sounds like it needs a little more work.

@PardeepSinghBali are you OK to review ?

Yes sure, I will

@PardeepSinghBali
Copy link
Contributor Author

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 edit-actions. See the original issue #324 and PR #325.

Screenshot 2025-03-19 at 2 41 46 pm

Screenshot 2025-03-19 at 2 52 34 pm

done and changes pushed

Copy link
Contributor
@andybroomfield andybroomfield left a 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.

@PardeepSinghBali
Copy link
Contributor Author

Thanks @PardeepSinghBali this is working as expected, and the id randomisation is applied to all iterations of edit-actions.

Perfect!

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.

4 participants
0