Skip to content

feat(select): add labelPlacement prop, justify prop, and rich content for select options#31241

Merged
thetaPC merged 11 commits into
major-9.0from
FW-7599
Jun 25, 2026
Merged

feat(select): add labelPlacement prop, justify prop, and rich content for select options#31241
thetaPC merged 11 commits into
major-9.0from
FW-7599

Conversation

@thetaPC

@thetaPC thetaPC commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Issue number: resolves #29890


What is the current behavior?

Select only allows plain text to be passed within it's options.

What is the new behavior?

ion-select-option

  • Added default slot support for custom HTML content (images, avatars, icons, etc.) when innerHTMLTemplatesEnabled is true
  • Added start and end named slots for content that appears in the overlay interface but not in the selected text
  • Added description prop for text rendered below the option label in the overlay
  • Add labelPlacement and justify props to ion-select-option and pass them through to alert, popover, and modal overlay paths
  • Add per-theme slot-size limitations so oversized slotted content is capped to keep option rows visually uniform within an overlay

ion-select

  • Selected text renders HTML content from the default slot, excluding start/end slot content
  • aria-label derives plain text only from the default slot, ensuring screen readers don't read slotted or element content
  • Added CSS variables for styling media within the selected text (--select-text-media-width, --select-text-media-height, etc.)

Overlay interfaces (alert, action-sheet, select-popover, select-modal)

  • All four interfaces render rich content (start, end, description, HTML label) consistently via the shared renderOptionLabel utility
  • Public interfaces (ActionSheetButton, AlertInput, SelectPopoverOption, SelectModalOption) are unchanged — rich content fields are on internal extended interfaces (SelectActionSheetButton, SelectAlertInput, SelectOverlayOption)
  • Content is sanitized via sanitizeDOMString before DOM injection to prevent XSS

Utilities

  • Added select-option-render.tsx: shared render utility for option labels across all overlay components
  • Added getOptionContent: extracts and clones slot content from ion-select-option for overlays and selected text
  • Added getDefaultSlotPlainText — extracts plain text from the default slot, used for labels and aria
  • Add reflectPropertiesToAttributes to core/src/utils/sanitization/ and call it from getOptionContent immediately before cloneNode. The helper mirrors a registered set of custom-element DOM properties (icon, name, src, ios, md on ion-icon) onto attributes so the cloned overlay copy renders correctly regardless of how the framework bound the prop.

Tests

  • Added E2E tests for rich content rendering across all four interfaces (single and multiple selection)
  • Added tests for slot positioning (start/end placement verification)
  • Added tests for selected text content verification (excludes slotted content)

Does this introduce a breaking change?

  • Yes
  • No

No, developers will be able to continue using plain text for select options as usual.

Other information

Dev build: 8.8.12-dev.11782342444.186b6e9c

Preview

@vercel

vercel Bot commented Jun 23, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Jun 25, 2026 6:23pm

Request Review

@github-actions github-actions Bot added package: core @ionic/core package package: angular @ionic/angular package package: vue @ionic/vue package package: react @ionic/react package labels Jun 23, 2026
@thetaPC thetaPC changed the title feat(select): add labelPlacement prop, justify prop, and rich content… feat(select): add labelPlacement prop, justify prop, and rich content for select options Jun 24, 2026
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@thetaPC thetaPC marked this pull request as ready for review June 24, 2026 23:15
@thetaPC thetaPC requested a review from a team as a code owner June 24, 2026 23:15
@thetaPC thetaPC requested review from ShaneK and gnbm and removed request for gnbm June 24, 2026 23:15

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking really good! Fairly minor change requests with one intention verification request

Comment thread core/src/utils/sanitization/index.ts Outdated
* form/navigation-hijack attributes (`formaction`, `action`, `target`),
* which are therefore stripped.
*/
const allowedAttributes = [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This allowedAttributes array is shared with sanitizeDOMString, so growing it from 6 entries to ~70 (plus aria-*/data-* wholesale) changes the sanitization output for every existing consumer, not just the new select path. ion-toast, ion-loading, the ion-alert message, ion-refresher-content, and ion-infinite-scroll-content all run innerHTML through this when innerHTMLTemplatesEnabled is on, so names like type, value, width, height, mode, and theme now survive where they were stripped before. The dangerous-vector handling is actually tighter than before so I don't think this is unsafe, but is the wider allowlist intended for those other components too? Worth a regression test covering one of them, and a line in the PR description so anyone bisecting a future toast/loading rendering difference knows where to look.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

const Tag = useSpan ? 'span' : 'div';
const keyPrefix = `${className}-${id}`;

sanitizeDOMTree(content);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

renderClonedContent calls sanitizeDOMTree(content) here, and the new "should sanitize an HTMLElement label that bypassed ion-select" spec shows this is the only sanitization pass for vanilla-JS callers that hand an HTMLElement straight to renderOptionLabel without going through getOptionContent. But the cloneToVNode comment just above says "pure structural conversion, no security filtering" and "Highly recommended to pre-sanitize the source DOM", which reads like sanitization always happens upstream. Someone could delete this call as redundant and reopen an XSS hole on the direct-HTMLElement path. Could you add a short note above it explaining that this is the only sanitize pass for direct HTMLElement callers and shouldn't be removed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@@ -23,6 +30,43 @@ export class SelectOption implements ComponentInterface {
*/
@Prop() value?: any | null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The disabled JSDoc just above (This property does not apply when interface="action-sheet" as ion-action-sheet does not allow for disabled buttons.) is now stale. This PR wires option.disabled through to the action-sheet button: createActionSheetButtons sets it, the button renders disabled={b.disabled}, there's new disabled styling in action-sheet.scss, and an e2e test covers it. The comment says the opposite of what ships now. Can you update it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ShaneK ShaneK left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! Great work 🚀

@thetaPC thetaPC merged commit a35e13d into major-9.0 Jun 25, 2026
50 checks passed
@thetaPC thetaPC deleted the FW-7599 branch June 25, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants