Skip to content

[985] Restore unique_id filter and use instead of Drupal's clean_unique_id#1012

Open
kmonahan wants to merge 2 commits into5.x-RCfrom
985-restore-unique_id
Open

[985] Restore unique_id filter and use instead of Drupal's clean_unique_id#1012
kmonahan wants to merge 2 commits into5.x-RCfrom
985-restore-unique_id

Conversation

@kmonahan
Copy link
Copy Markdown
Collaborator

Fixes #985

@kmonahan kmonahan changed the base branch from 5.x to 5.x-RC March 25, 2026 13:14
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

A custom unique_id filter is reintroduced across both JavaScript and PHP implementations, replacing the previously-used clean_unique_id filter. A new Drupal Twig extension class is added with a unique_id filter that sanitizes input identifiers and appends random Base64 strings. The JavaScript library is updated to export a renamed uniqueId function that registers a corresponding filter. Multiple template files are updated to use the unique_id filter instead of clean_unique_id for generating element identifiers in components, forms, and layout templates.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: restoring the unique_id filter and switching from Drupal's clean_unique_id.
Description check ✅ Passed The description references issue #985, which is directly related to the changeset that restores the unique_id filter.
Linked Issues check ✅ Passed The PR fully implements the objective from #985: it restores the custom unique_id filter as a Twig extension, registers it in services, and replaces all uses of clean_unique_id with unique_id across components and templates.
Out of Scope Changes check ✅ Passed All changes are directly scoped to restoring and implementing the unique_id filter as required by #985; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 985-restore-unique_id

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/uniqueId.js (1)

5-9: ⚠️ Potential issue | 🟠 Major

Align JS unique_id output with Drupal sanitization behavior.

On Line 6, the raw value is used as the ID prefix. Drupal’s server-side filter sanitizes first (Html::getId($id)), so Storybook can generate different/invalid IDs for the same input.

Proposed fix
 function uniqueId(twigInstance) {
+  const sanitizeId = input =>
+    String(input ?? '')
+      .toLowerCase()
+      .trim()
+      .replace(/[\s_]+/g, '-')
+      .replace(/[^a-z0-9-]/g, '-')
+      .replace(/-+/g, '-')
+      .replace(/^-|-$/g, '') || 'id';
+
   // unique ID generator via https://stackoverflow.com/a/48593447
   twigInstance.extendFilter(
     'unique_id',
     value =>
-      `${value}--${(Date.now() * 1000 + Math.random() * 1000)
+      `${sanitizeId(value)}--${(Date.now() * 1000 + Math.random() * 1000)
         .toString(16)
         .replace(/\./g, '')
         .padEnd(14, '0')}`
   );
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/uniqueId.js` around lines 5 - 9, The prefix `value` is used raw when
building the ID, causing mismatch with Drupal's Html::getId sanitization; update
the unique id generator in lib/uniqueId.js to sanitize the `value` the same way
Drupal does before concatenating the timestamp suffix (e.g., normalize to ASCII,
lowercase, replace non-alphanumeric characters with '-', collapse/trim
leading/trailing '-' and ensure it doesn't start with a digit or empty — add a
fallback prefix if needed), then use that sanitizedPrefix in place of `value` in
the template string so Storybook IDs match Drupal-generated IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@lib/uniqueId.js`:
- Around line 5-9: The prefix `value` is used raw when building the ID, causing
mismatch with Drupal's Html::getId sanitization; update the unique id generator
in lib/uniqueId.js to sanitize the `value` the same way Drupal does before
concatenating the timestamp suffix (e.g., normalize to ASCII, lowercase, replace
non-alphanumeric characters with '-', collapse/trim leading/trailing '-' and
ensure it doesn't start with a digit or empty — add a fallback prefix if
needed), then use that sanitizedPrefix in place of `value` in the template
string so Storybook IDs match Drupal-generated IDs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1292c3c1-5d73-4418-aede-328f1e5c90e4

📥 Commits

Reviewing files that changed from the base of the PR and between a8bb2b2 and 7cf4f6a.

📒 Files selected for processing (11)
  • .storybook/preview.js
  • gesso_helper/gesso_helper.services.yml
  • gesso_helper/src/TwigExtension/UniqueIdTwigExtension.php
  • lib/uniqueId.js
  • source/03-components/accordion/accordion-item.twig
  • source/03-components/overlay-menu/overlay-menu.twig
  • source/03-components/pager/pager--mini/pager--mini.twig
  • source/03-components/side-menu/side-menu.twig
  • source/03-components/tabs/tabs.twig
  • templates/form/form.html.twig
  • templates/layout/html.html.twig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Bring back unique_id

2 participants