fix(core): expose bodyRole + bodyAriaLabelledBy inputs on fd-popover (#14260)#14270
Open
droshev wants to merge 13 commits into
Open
fix(core): expose bodyRole + bodyAriaLabelledBy inputs on fd-popover (#14260)#14270droshev wants to merge 13 commits into
droshev wants to merge 13 commits into
Conversation
droshev
changed the title
✅ Deploy Preview for fundamental-ngx ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
droshev
changed the title
droshev
force-pushed
the
fix/popover-body-role-input
branch
from
5594e71 to
2c1e0c1
Compare
♿ Accessibility Report✅ 958/958 routes passed axe-core audit |
| * | ||
| * See https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/ for guidance. | ||
| */ | ||
| readonly bodyRole = input<string | null>('dialog'); |
Contributor
There was a problem hiding this comment.
Right now the API documentation suggests a specific set of supported values ('dialog', 'region', 'menu', 'listbox', 'tooltip', null), but the type is string | null, which means consumers get no autocomplete, no compile-time validation, and can pass invalid values.
If those are indeed the only supported values, a union type would improve the developer experience:
type PopoverBodyRole = 'dialog' | 'region' | 'menu' | 'listbox' | 'tooltip';
readonly bodyRole = input<PopoverBodyRole | null>('dialog');
Contributor
Author
There was a problem hiding this comment.
Added PopoverBodyRole union type with the 6 most common roles + (string & {}) escape hatch for edge cases like 'tree', 'status', 'alert'. Gives autocomplete without gatekeeping the ARIA spec. Also updated PopoverConfig.bodyRole for consistency
…#14260) Bind [bodyAriaLabel] using i18n key coreFormInputMessageGroup.popoverAriaLabel. Requires CRUCIBLE Task 2.2 (i18n key registration) for full compilation.
…14260) Bind [bodyAriaLabel] using i18n key platformInputMessageGroup.popoverAriaLabel. Requires CRUCIBLE Task 2.2 (i18n key registration) for full compilation.
… popoverAriaLabel keys (#14260)
The nullish-coalescing fallback chain bodyRole() ?? cfg.bodyRole ?? 'dialog'
swallowed null, making it impossible for consumers to opt out of role=dialog
via [bodyRole]='null'. Trust the input value as authoritative — its default
('dialog') already handles the unbound case.
Selectors targeted the <fd-popover-body> host element, but [attr.role], [attr.aria-label], [attr.aria-labelledby] are bound on the inner .fd-popover__body div per popover-body.component.html:14-16. Wrong selector returned the host (no role attr) and getAttribute returned null.
droshev
force-pushed
the
fix/popover-body-role-input
branch
from
0ac89c0 to
5aea2b8
Compare
Verify union values and escape hatch both work correctly.
droshev
force-pushed
the
fix/popover-body-role-input
branch
from
5aea2b8 to
422486e
Compare
InnaAtanasova
approved these changes
Jun 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14260
Summary
Exposes
bodyRoleandbodyAriaLabelledByas template inputs on<fd-popover>so consumers can set the correct ARIA role without going through the imperativePopoverConfigpath.Background: PR #12907 changed
PopoverBodyComponent._bodyRoledefault fromnull→'dialog'to fix screen reader announcements. However,bodyRolewas only settable viaPopoverConfig(imperative API). Template consumers (<fd-popover>in HTML) had no way to override it.This PR: Adds template inputs so all consumers can set the correct role:
Default behavior unchanged. All 94 existing popover consumers remain byte-identical:
bodyRoledefaults to'dialog'.nullis reachable —[bodyRole]="null"renders no role attribute at all, which is the correct ARIA shape for non-modal disclosure widgets where the trigger'saria-haspopupalready carries the relationship.Changes
Primitive surface (
<fd-popover>)bodyRole = input<PopoverBodyRole | (string & {}) | null>('dialog')— new input with type union for autocomplete + escape hatchbodyAriaLabelledBy = input<string | null>(null)— new inputPopoverBodyRoletype exported:'dialog' | 'region' | 'menu' | 'listbox' | 'tooltip' | 'alertdialog'_effectiveConfig→PopoverService.refreshConfiguration→PopoverBodyComponentsignalsbodyRoleread path trusts the input directly (no?? cfg ?? 'dialog'fallback chain — that would swallow explicitnull). Sibling inputs (bodyAriaLabel,bodyAriaLabelledBy) keep the chain since their default isnulland fall-through is meaningful.Consumer patches
fd-form-input-message-group: binds[bodyAriaLabel]to new i18n keycoreFormInputMessageGroup.popoverAriaLabelfdp-input-message-group: binds[bodyAriaLabel]to new i18n keyplatformInputMessageGroup.popoverAriaLabelOut of scope (filed separately)
A follow-up audit will (a) classify the remaining 92 popover consumers by ARIA semantics (disclosure →
null, listbox →'listbox', menu →'menu', region →'region'+ name, true dialog →'dialog'+ name) and (b) flipbodyRole's default from'dialog'tonull. Bundling that into this PR would balloon the diff and force a 94-consumer baseline regen.