Repository navigation
fix(core): guard null anchor role in public menu - #65706
bulungasdeveloper wants to merge 1 commit into
Conversation
|
Thanks! The PR is now rebased on current master and the change itself is green: The remaining red checks are environmental:
Could someone with write access re-run the Playwright workflow (all jobs)? |
bd9c9b5 to
80f6f58
Compare
80f6f58 to
26b3a36
Compare
|
Friendly ping for review. All required checks are green ( The PR is waiting for two approvals. Could a frontend reviewer take a look when Thanks! |
| const anchor = document.getElementById(`${props.id}--link`) as HTMLAnchorElement | ||
| const anchor = document.getElementById(`${props.id}--link`) as HTMLAnchorElement | null | ||
| if (!anchor) { | ||
| return | ||
| } |
There was a problem hiding this comment.
but this then must be retried later if this is a race condition, even better to delay mounting until its available.
There was a problem hiding this comment.
Thanks for the feedback. You’re right – returning early only hides the race. I’ve updated the component to retry setting the role until the anchor is actually in the DOM.
PublicPageMenuEntry.vue now uses a small requestAnimationFrame retry loop in onMounted:
- it tries to find document.getElementById(${id}--link)
- if it’s missing, it schedules another attempt, up to 10 frames
- once the anchor appears, role="menuitem" and the optional clickOnly handler are applied
This avoids the TypeError: Cannot set properties of null without bailing out, and it still does nothing if the anchor never appears. The test remains green and the change is backward compatible.
Commit 3247050 pushed to fix-publicpagemenu-null-role.
|
Rebased onto latest master (fe381b4) so the checks run against a current base. All CI checks are green except The retry logic for the anchor was added as suggested: @susnux could you take another look when you have a moment? |
The anchor rendered by NcListItem may not be in the DOM at onMounted time, causing a race. Retry with requestAnimationFrame for up to 10 frames instead of bailing out immediately. Assisted-by: opencode:opencode Signed-off-by: Braux Martinez <martinezjb@hitss.com>
3247050 to
1cba555
Compare
Summary
PublicPageMenuEntryassumes the anchor rendered byNcListItemisalready in the DOM when it mounts. If the entry mounts first,
document.getElementById()returnsnullandanchor.role = 'menuitem'throws a
TypeError, aborting the mount and leaving the public page menuwithout entries.
Return early when the anchor is missing. Nothing changes when it exists.
How to test
/s/<token>).TypeError ... setting 'role'; the menu keeps its entries and theanchors have
role="menuitem".The unit test mounts the component with the anchor missing and asserts
that it does not throw.
Checklist
eslint)PublicPageMenuExternalEntry.spec.ts, 3/3)AI