From 7bd4a644b7720ed803541219db1cb53eed86bfe5 Mon Sep 17 00:00:00 2001 From: Benny Powers Date: Mon, 4 May 2026 15:16:14 +0300 Subject: [PATCH 1/4] docs: plans for pfv6 --- .claude/ADVICE.md | 1237 ++++++++++++++++++++++++ .claude/V6-MIGRATION-PLAN.md | 198 ++++ .claude/settings.json | 21 + .claude/skills/create-element/SKILL.md | 287 ++++++ .claude/skills/review-a11y/SKILL.md | 196 ++++ .claude/skills/review-api/SKILL.md | 174 ++++ .claude/skills/review-demos/SKILL.md | 134 +++ .claude/skills/update-element/SKILL.md | 268 +++++ .gitignore | 2 + CLAUDE.md | 118 +++ 10 files changed, 2635 insertions(+) create mode 100644 .claude/ADVICE.md create mode 100644 .claude/V6-MIGRATION-PLAN.md create mode 100644 .claude/settings.json create mode 100644 .claude/skills/create-element/SKILL.md create mode 100644 .claude/skills/review-a11y/SKILL.md create mode 100644 .claude/skills/review-api/SKILL.md create mode 100644 .claude/skills/review-demos/SKILL.md create mode 100644 .claude/skills/update-element/SKILL.md create mode 100644 CLAUDE.md diff --git a/.claude/ADVICE.md b/.claude/ADVICE.md new file mode 100644 index 0000000000..1fbe778d51 --- /dev/null +++ b/.claude/ADVICE.md @@ -0,0 +1,1237 @@ +# API Design Advice + +Distilled from Benny Powers' PR reviews across 2800+ PRs in RedHat-UX/red-hat-design-system and 2000+ PRs in patternfly/patternfly-elements. + +## HTML + +### Attributes & Properties + +#### Prefer enum-style `variant` attributes, over multiple booleans +Consolidate mutually exclusive visual states into a single `variant` attribute with an enum type rather than multiple boolean attributes. Variants should be exclusive, not stackable. + +> "this line introduces a new concept to the design system: multiple layered variants. prior to this, variants were exclusive. this change means that an element can have 0-or-more variants at one time. this opens up quite a bit of flexibility for users, however it could introduce explosive complexity and additional cognitive overhead." (RHDS PR #1587) + +Recurring pattern: RHDS PRs #952, #1587 + +#### Distinguish `variant` from theme attributes +`variant` (or `type`) is for functional/structural differences between component configurations. Theme attributes are for color/theme differences. Never conflate the two. + +> RHDS PR #288, #486 + +#### Boolean attributes for boolean states +When an attribute is yes/no (present/absent), use a boolean attribute, not a string `"true"/"false"`. Use `@property({ type: Boolean })` with a default of `false`. + +> "does this need to be `'true'|'false'`? shouldn't it then be a boolean attr?" (RHDS PR #634) + +#### Avoid `is-` prefix for boolean attributes +Since boolean attributes are true when present and false when absent, the `is-` prefix adds noise without value. Pair dash-case attributes to camelCase properties. + +> "As the presence of boolean attributes indicate true and their absence false, and since we pair dash-case attributes to camelCase properties, we've avoided the syntax noise of `is-` prefixes for boolean element flags." (PFE PR #2357, #2045) + +#### Don't default reflected booleans to `true` +A reflecting boolean attribute that defaults to `true` is problematic because there is no good way to unset it in HTML. Flip the sense of the attribute. + +> "A reflecting boolean attr that defaults to true isn't great because there will be no good way to unset. Maybe flip (zing!) the sense to `no-flip`?" (PFE PR #2320) + +#### Avoid initializing reflected properties +Don't set defaults on reflected properties. Let absence be the default state to avoid noisy attributes appearing on every instance when they connect to the DOM. + +> "Let's avoid initializing reflected properties. We also don't really need the default state, we can just assume that as the default." (PFE PR #2186) + +#### Don't reflect array properties +Avoid accepting and certainly reflecting array properties. If a list is needed, use a converter (like NumberListConverter). + +> "We should avoid accepting and certainly reflecting array props. If this *must* be a list, use a converter like the NumberListConverter." (PFE PR #2320) + +#### Don't sprout attributes +Avoid dynamically adding attributes that weren't present at creation time. This is an antipattern per the spec editor. + +> "This way we avoid 'sprouting' attrs, which we've seen is an antipattern acc'd to the spec editor." Cited whatwg/html#8365. (PFE PR #2183) + +#### Boolean-or-enum converter for multi-modal attributes +When an attribute has a default boolean behavior but also supports specific modes, use a boolean-or-enum converter pattern. Create reusable converter functions for patterns that appear in multiple elements. + +> Consider `` instead of `` (RHDS PR #1239) + +#### Attributes should always have sensible defaults +All properties/attributes should have defaults. An element should work correctly with zero attributes set. + +> "should have a default" (RHDS PR #952, recurring) + +#### Multi-word attributes use dash-case +Multi-word attribute names should be dash-case since HTML attributes are case-insensitive. Property names should be camelCase. + +> "The attr for this should be `full-height`." (PFE PR #2222); "We should prefer dash-case attributes for camelCase DOM properties, i.e. `date-format-input`, since HTML attributes are case-insensitive." (PFE PR #2599) + +#### Don't expose internal state as public attributes +Internal implementation details like breakpoint state, color context, or responsive behavior should not be exposed as attributes. Use controllers internally. + +> Removed `is-mobile` attribute (RHDS PR #471); made `on` attribute shadow-private across RHDS PRs #660, #661, #675 + +#### Don't expose `aria-*` as public API +Custom elements should expose their own semantic attributes (e.g., `disabled`, `accessible-label`) and manage ARIA attributes internally via ElementInternals. + +> "we should prefer our own `disabled` attr, not expose `aria-` as public apis" (RHDS PR #1239); "Users shouldn't set aria on their own." (PFE PR #2699) + +#### Abstract ARIA behind custom attributes +Provide custom attributes for screen-reader names rather than exposing ARIA attributes directly. Apply `aria-label` and `role` via ElementInternals internally. This simplifies the public API and reduces user error. + +> "Both the aria-label and role will be set by element internals. Easier for the end user and less typing." (RHDS PR #1732) + +#### Provide attribute/slot pairs for content +For content that could be plain text or rich markup, provide both an attribute (for simple use) and a named slot (for rich content). Demos should primarily show the attribute form. + +> "I noticed these are all plain text, which indicates that these should be slot/attribute pairs; the slot being preferred for rich content and the attribute being preferred for plain text." (RHDS PR #2802) + +#### Internalize accessibility wiring via attribute/slot pairs +If content serves as an accessible description (helper text, error text), make it an attr/slot pair so the component handles `aria-describedby` automatically, removing that burden from users. + +> "If the design for this permits, we should consider making this an attr/slot pair, and that should allow us to remove the requirement for users to add `aria-describedby`" (RHDS PR #2802) + +#### Normalize enumerated attribute values +Always normalize enumerated attribute values to lowercase internally, so `state="neUtRal"` works as expected. + +> RHDS PR #2484 + +#### Remove dead attributes promptly +If an attribute has no effect (no CSS or JS references it), remove it with a proper major version bump. Dead attributes mislead users. + +> "remove `bordered` from accordion. it anyways doesn't do anything" (RHDS PR #2217) + +#### Don't redeclare native HTML attributes +The `role` attribute already exists on all elements. Don't add a `@property` for it. + +> "the role attribute is native, and already reflects" (RHDS PR #2605) + +#### Replace custom attributes with `data-action` pattern +Instead of inventing custom boolean attributes for action buttons (like `dismiss`, `cancel`), use `data-action="dismiss|cancel"`. This avoids potential conflicts with future standard attributes. + +> RHDS PR #267 + +#### Use container queries over manual compact/responsive variants +If a layout change is purely a response to available space, make it automatic via container queries. Document how users can force the narrower layout by constraining the container width. + +> "compact variant should be replaced with a container query, and we should document how to force it into compact layout by putting it in a narrow container" (RHDS PR #1587) + +#### Avoid boolean `has-*` attributes +Instead of boolean `has-*` attributes that signal "some CSS property is set," prefer value-carrying attributes. + +> "can we either remove this attribute or give it a value e.g. ``" (RHDS PR #679) + +#### Prefer opt-in over opt-out for features +Reverse the sense of opt-out booleans so features are opt-in by default. + +> "Can we reverse the sense of this? So that filter is opt in instead of opt out?" (PFE PR #2570) + +#### Avoid empty strings as public API values +Empty string values are unclear in a public API. Prefer explicit, meaningful values and handle empty string at the access sites. + +> "The empty string here... it's not great for a public API, since it's not immediately clear what `''` means here, why I'd need to use it, or what will happen if I do." (PFE PR #2570) + +#### Limit public custom elements per component +Prefer working within a single shadow root when possible. Only ship multiple elements when accessibility or composition requires it. + +> "Since upstream only provides the `DatePicker` react component, we should also try to limit the number of public components we ship here." (PFE PR #2599) + +### Slots + +#### Default slot for primary content +The main content of a component goes in the unnamed (default) slot. Only name slots for secondary/metadata content areas. + +> "Giving this some thought, does this need to be named?" (RHDS PR #377, recurring across RHDS PRs #267, #471) + +#### Use semantic slot names, not implementation-specific ones +Slot names should reflect semantic purpose. Prefer `actions` over `action-groups`, `header` over `title`. Prefer brevity: `actions` over `action-links`, `category` over `category-label`. + +> "These are in keeping with the conventions we've developed in PFE" (RHDS PR #267); PFE PRs #2949, #2955 + +#### Use the DOM standard term "default slot" +The DOM standard uses the term "default slot", not "anonymous slot". + +> "The DOM standard uses the term 'default slot' instead of anonymous. We might want to adopt that." (PFE PR #2593) + +#### Require semantic HTML in slots +Header slots should accept `

`-`

` tags. Design slot APIs around standard HTML semantics rather than requiring custom class names. + +> "Prefer h2-6 tags for header, in keeping with semantic HTML" (RHDS PR #267) + +#### Don't provide placeholder content in slots +End users should never see strings like "Placeholder Description" on pages where developers forgot to include required content. Issue console warnings instead. + +> "I removed the 'Placeholder Description' content, because I don't want end users to see strings like that on pages where developers forgot to include required content." (RHDS PR #471) + +#### Be conservative about adding named slots +Consider alternatives (attributes, i18n controllers) when the number of slots could grow unboundedly. + +> "We could always add another slot, but I'm wary that we might end up with dozens of slot names for the various states" (RHDS PR #1496) + +#### Use named slots with defaults for i18n +Allow translation of built-in UI text by providing named slots with English default content. + +> RHDS PR #591 + +#### Hide empty slot containers +When a slot has no slotted content, hide wrapper/container elements in shadow DOM to avoid empty spacing, visual artifacts, or empty landmarks in the accessibility tree. When ::has-slotted() becomes baseline, use it, until then, +it requires use of the DOM API to query the element's children client-side. SSR scenarios then require a more advanced solution which parses inputs and places `ssr-hint-has-slotted` attributes. + +> Recurring pattern: RHDS PRs #1558, #1895, #2646 + +#### Don't add slots tightly coupled to a single child component +Prefer composable, general-purpose slots. If the existing API can support the use case (even with CSS workarounds), prefer that over API expansion. + +> RHDS PR #1622 + +#### Keep slot semantics strict +Don't mix unrelated content types in a slot designed for a specific purpose. Design composable patterns where elements link across the DOM via idref references instead. + +> "This is maybe a case for disconnected tabs (``)..." (RHDS PR #995) + +#### Prefer text nodes and attributes over nested child elements +For simple content with metadata, use text nodes for content and attributes for metadata. + +> "can we do cues like this? `I'm Burr Sutter.`" (RHDS PR #679) + +#### Use prescriptive slot descriptions in JSDoc +Slot descriptions should tell users what to put there, not describe the slot abstractly. + +> "A prescriptive explanation would be better here: `@slot - Label text`" (PFE PR #1899) + +### Shadow DOM & Templates + +#### Minimize wrapper/container elements +If a class or attribute can be applied directly to a semantic element (like `
`), don't add an extra `
` wrapper. Fewer DOM nodes mean better performance and simpler CSS. + +> "we can remove the container and move the classmap to the article" (RHDS PR #952, recurring with #553) + +#### Prefer `` directly over wrapping `
` +Since slots have `display: contents`, slotted content participates in the parent's layout. Only add wrapper divs when CSS layout truly requires them. + +> "Slot has display: contents by default which means that as far as CSS layout is concerned, it's as if the slotted content nodes are direct children of the div." (RHDS PR #1302); "Do we need the wrapper div? Why not `` and set `display: block`?" (PFE PR #2588) + +#### Set display on slot elements instead of wrapping divs +When a slot is wrapped in a div purely for layout, try setting display directly on the slot element. + +> "consider removing the wrapping div and setting a display property on the slot" (RHDS PR #1197) + +#### Use ID selectors and tag selectors in shadow DOM CSS +Prefer `#id` and `tagname` selectors over `.class` in shadow roots. Shadow DOM provides encapsulation, so IDs are safe and more semantic. + +> RHDS PR #267, #377 (recurring) + +#### No BEM in shadow DOM +BEM conventions were designed for a global CSS paradigm. Shadow DOM provides encapsulation, making BEM unnecessary and noisy. Use IDs for unique elements. + +> "We shouldn't use BEM in shadow DOM." (PFE PR #2173); "internal dom does not and should not follow BEM conventions which were intended and implemented for a global css paradigm." (PFE PR #2155) + +#### No classes required in light DOM +Components should not require users to add specific CSS classes to light DOM content. + +> "remove the requirement to use specific classes in light DOM" (RHDS PR #267) + +#### Use `::slotted()` styles over light DOM stylesheets +When possible, style slotted content from shadow DOM using `::slotted()` rather than requiring light DOM stylesheets. + +> "Couldn't we do this in shadow DOM through the magic of `:is`?" (RHDS PR #254) + +#### Distinguish slotted vs fallback content in CSS +Regular selectors target fallback content; `::slotted()` targets slotted content. Combine them to cover both cases. + +> "you might want `rh-icon, ::slotted(rh-icon[slot=icon]) {` just to be safe. test it both ways" (RHDS PR #2604) + +#### No self-closing void elements +Follow the HTML spec strictly: ``, `
`, `` should not have trailing slashes. + +> "void elements shouldn't have a slash" (RHDS PR #1197) + +#### Static content belongs in HTML, not JavaScript +Static modal/dialog content should be in the HTML template, not created dynamically in JavaScript. + +> "This modal content is static, so put it in the HTML rather than in javascript, and call showModal() on it when you need to." (PFE PR #2949) + +## CSS + +### Custom Properties & Parts + +#### Private CSS custom properties use `--_` prefix +Internal/private CSS custom properties use `--_` prefix. Public/customizable ones use component-namespaced names. + +> Recurring pattern: RHDS PRs #436, #553, #1330, #2449; PFE PRs #2948 + +#### Don't create custom properties that duplicate native CSS +If users can achieve the same thing with `width`, `color`, etc., don't add a component-specific custom property for it. + +> "although tbh `--rh-spinner-width` might be superfluous. Why not just make the svg's width 100% and have users set built-in property `width` on the host?" (RHDS PR #553) + +#### Less is more for CSS custom property APIs +Only expose custom properties when they provide value beyond native CSS. You can always add them later. + +> "better don't expose min-width as a token at all, unless specifically needed, users could just set `min-width` themselves" (RHDS PR #599) + +#### Consider CSS custom properties before adding variant attributes +If a visual change is purely stylistic (e.g., transparent backgrounds), a CSS custom property is lower-cost and more flexible than a new attribute. + +> "`open` might just be a pattern where the user sets `--rh-pagination-control-background-color` to `transparent`" (RHDS PR #1587) + +#### Question the need for CSS parts aggressively +Parts create API surface that must be maintained and can allow users to break component internals. Only expose parts with clear use cases. + +> "I don't want to get into a sticky situation here. The design guidelines were pretty clear, and a lot of the functionality is tied to the css. Opening up parts could easily break stuff in this case." (RHDS PR #279); "In general, we try to avoid adding parts unless we're sure there's a need for them." (PFE PR #2955) + +#### Public CSS properties should be declared as fallback defaults +Public properties should be provided as fallback defaults at the use site, not set on `:host`. Setting on `:host` overrides user settings. + +> RHDS PR #553; "Instead of setting these on `:host`, which would override any user settings, please make them fallbacks." (PFE PR #2497); "Instead of actively assigning these on :host, can we just pass the fallback values at the callsites? Less convenient for maintainers but probably better for users, since they'll be able to override when theming." (PFE PR #2114) + +#### Non-overridable CSS properties go on internal elements +CSS custom properties that are intentionally non-overridable should be set on internal shadow DOM elements, not the host. + +> "For those which are intentionally meant to override user configuration, those should be set on the svg element." (PFE PR #2155) + +#### Distinguish system tokens from component tokens +System-level tokens come from the design token package and are used as-is. Component-level custom properties need full `@cssprop` documentation. + +> See [RHDS-Specific Conventions: Token System](#token-system) for RHDS token naming. + +#### Use token names, not raw values, everywhere +In code, reviews, and design specs, always reference design token names rather than pixel/hex values. Token values may change centrally. + +> "The values are not as important as the token names. The values we'll automatically fix later, so if the token names are ok, just approve." (RHDS PR #489, recurring) + +#### T-shirt sizes must map to established token values +Don't redefine what "xl" means per-element. XL should mean the same thing across the system. + +> "This is still confusing. We have length tokens for a reason, why are we intentionally ignoring them? XL should be the same thing in all cases." (RHDS PR #2430) + +#### CSS docstrings describe token usage within the element +When documenting CSS custom properties, describe how the token is used in this specific element, not the token's general purpose. + +> "docstrings here will be prepended to the design tokens docstrings, so our text should describe the token's use *within this element*" (RHDS PR #2858) + +#### Use logical properties in CSS custom property names +Use `border-block-start` not `border-top` in custom property naming. + +> RHDS PR #1720 + +#### Attribute selectors are a smell for theming +Theming should be done via CSS custom properties, not via attribute selectors on theme attributes. + +> "attribute selectors are a smell and shouldn't be required for any theming operation" (RHDS PR #1330) + +### Conventions + +#### Always use CSS logical properties +Use `inline-start`/`block-start` instead of `left`/`top` to support RTL and vertical writing modes. + +> "Please use `inline-start` instead of `left`" (RHDS PR #377, recurring across RHDS PRs #259, #2605) + +#### Use `::after` double-colon syntax for pseudo-elements +Per CSS3 spec. + +> "`:after` works, but CSS3 prefers `::after`" (RHDS PR #259) + +#### Animate GPU-friendly properties +Animate `transform`/`translate`/`opacity`, not `padding`/`margin`/`width`. + +> "The change I made was to animate the `translate` value instead of the `padding` value, which is much more performant" (RHDS PR #349) + +#### Shadow DOM animation names don't need prefixes +CSS animation names within shadow DOM are scoped automatically. Keep them short and readable. + +> "this animation name doesn't need a prefix, it's encapsulated in the shadow dom" (RHDS PR #553) + +#### Use modern CSS transform properties +Use individual transform properties (`rotate`, `translate`, `scale`) instead of the combined `transform` function. + +> "I think we should already prefer transform properties: `rotate: var(...)`" (PFE PR #2114) + +#### Use `aspect-ratio` instead of width+height +When width and height should maintain a ratio, use the `aspect-ratio` property. + +> Refactored spinner CSS "to use `aspect-ratio` instead of setting width and height." (PFE PR #2155) + +#### Use `pointer-events: none` on disabled hosts +Disabled elements should prevent interaction at the CSS level. + +> `:host([disabled]), :host(:disabled) { pointer-events: none; }` (PFE PR #2221) + +#### Use CSS grid over CSS table modes +CSS grid handles spanning (like `colspan`) better than CSS table display modes. + +> "We should consider using css-grid in place of table modes. I haven't found an effective means of emulating `colspan` with table modes... whereas with grid, we can just `grid-column: 1/-1`." (PFE PR #2564) + +#### Use native CSS nesting +Prefer native CSS nesting syntax over preprocessor nesting. + +> "We generally prefer to use nesting syntax." (PFE PR #2955) + +#### Don't nest `:host` attribute selectors with `&` +CSS nesting does not support `&([attr])` as a shorthand for `:host([attr])`. The `&` in nested CSS represents the parent selector as-is, but `:host` is a pseudo-class function, and `&([attr])` parses as a function call, not an attribute selector appended to `:host`. Keep `:host([variant="..."])` blocks flat at the top level. Nesting is valid for descendant selectors within those blocks, e.g. `& button:hover` or `:host([disabled]) &`. + +```css +/* WRONG: &([attr]) is invalid */ +:host { + &([variant="primary"]) { ... } +} + +/* RIGHT: flat :host selectors, nested descendants */ +:host([variant="primary"]) { + --_color: blue; +} + +:host([variant="primary"]) button { + &:hover { --_color: darkblue; } + &:active { --_color: navy; } +} +``` + +#### Use `display: contents` cautiously +`display: contents` on `:host` can cause issues. Consider ResizeObserver with a private CSS var instead. + +> "More and more I'm thinking that we should avoid `:host { display: contents; }`, and maybe calculate a private css var in resize observer instead." (PFE PR #2630) + +## JavaScript + +### Events + +#### Use custom `Event` subclasses, not `CustomEvent` with detail +Create typed Event subclasses instead of `new CustomEvent('name', { detail: {...} })`. +Better TypeScript and runtime type safety and cleaner APIs. +If event detail contains data, use class fields on the event, e.g. + +```ts +export class DialogCloseEvent extends Event { + reason: string; + constructor(reason: string) { + super('close', { bubbles: true, cancellable: true }); + this.reason = reason; + } +} +``` + +> Also in PFE: "Can we deprecate the el:name events and add our own `extends Event` events?" (PFE PR #1829); "Prefer to extend `Event` with `bubbles: true` and optionally `cancellable: true`, rather than using `ComposedEvent`." (PFE PR #2599) + +#### Make destructive/state-changing events cancelable +Events for close, delete, copy, and similar actions should be cancelable via `preventDefault()`. Demonstrate the cancelable pattern in demos. + +> "allow handlers to prevent close with `event.preventDefault()`" (RHDS PR #595) + +#### Make action events mutable +Events for user-facing actions (like copy) should allow consumers to modify the action data, not just cancel. + +> RHDS PR #2677 -- copy event allows modifying copied text (e.g., removing shell prompts) + +#### Prefer simple native Event objects when no data is needed +Use `new Event('ready')` rather than CustomEvent when the event carries no data payload. + +> "I think we can use a `new Event('ready')` here" (RHDS PR #436) + +#### Match native event names when wrapping native elements +When wrapping `` or similar, match native event names and timing. Users expect custom elements to behave like their native counterparts. + +> RHDS PR #1474 + +#### Don't bake analytics into components +Let analytics scripts add their own event listeners. This reduces bundle size for non-analytics users. + +> "The alternative is IMO much better: analytics users implement their own analytics." (RHDS PR #349) + +### Architecture + +#### Prefer forking over inheriting from upstream base classes +Don't force inheritance when the downstream component has significantly different needs. Extend LitElement directly for cleaner, simpler code. Share controllers and helpers without class inheritance. + +> "We should consider extending LitElement and reimplementing the DOM here: I'm not sure what advantage we get by extending BaseCard." (RHDS PR #952, recurring with #550) + +#### Prefer composition (controllers) over inheritance (base classes) +Controllers allow sharing behavior without class hierarchy. Configuration is better than inheritance for shared behavior. + +> "Since I was able to put everything in the controller, we don't actually export a base class... composition > inheritance" (PFE PR #2283); "Better to config then inherit." (PFE PR #2375) + +#### Elements should have single responsibility +If combining features creates accessibility issues, split them. Let consuming patterns compose simple primitives. + +> "remove all the dropdown/button/floating-ui stuff from menu... reimplemented that stuff in audio-player... this fixes cross-root aria problems" (RHDS PR #778) + +#### Context propagation must work at arbitrary depth +Test context with grandchild and deeply nested scenarios, not just parent-child. + +> RHDS PR #1985 + +#### Lazy-load optional dependencies +When a component optionally uses another component (e.g., an icon), lazy-load that dependency only when the relevant attribute is set. + +> "add something like `@observes('icon') protected iconChanged() { if (this.icon) { import('@rhds/elements/rh-icon/rh-icon.js'); } }`" (RHDS PR #2604) + +#### Don't import heavy token maps in client code +Use CSS custom properties and `getComputedStyle` to access values at runtime, keeping bundle size small. + +> "We can't use this in client-facing code, it'll drag in the entire map, which is just too much js to justify the convenience of `.get`." (RHDS PR #436) + +#### Move private statics to module scope +Private static methods should be module-scoped functions rather than static class methods, to prevent users from seeing them on the constructor. + +> "move private static methods into module scope to prevent users from being confused" (RHDS PR #2170) + +#### SSR awareness in component code +DOM APIs like `getBoundingClientRect()`, `children`, `assignedElements()` are not available during server-side rendering. Guard DOM-dependent code with `isServer` checks. + +> "this might crash in ssr. better to wrap the whole function body in if isServer" (RHDS PR #2448, recurring with #2605, #2802) + +#### Prefer declarative state over imperative +Prefer attribute-driven state over imperative state set via JS methods. More predictable and compatible with SSR. + +> RHDS PR #2429 + +#### Prefer re-export entrypoint modules over pre-built bundles +Preserves tree-shaking while providing convenience. + +> RHDS PR #1630 + +#### Use MutationObserver when slotchange is insufficient +`slotchange` won't fire when children are added/removed without slot attributes changing. Use MutationObserver for dynamic content. + +> RHDS PR #279 + +#### `slotchange` already bubbles — don't add redundant listeners +Don't manually add `slotchange` listeners on individual slots when you can listen on the shadow root. + +> "Not needed, slotchange already bubbles." (PFE PR #2563) + +#### Be careful with `hostConnected` +`hostConnected` reinitializes state when elements are moved in the DOM. State set in `hostConnected` will be blown away. + +> "This will blow away the state... `$('#other-container').append(el); // oops... hostConnected()`" (PFE PR #2410) + +#### Use single static event listeners instead of per-instance +For document-level event listeners, add a single static listener that iterates over a private set of element instances. Per-instance listeners are memory leaks. + +> "This is a memory leak. We must only add one listener to the document, in a static initialization block, and have that listener iterate over a private set of element instances." (PFE PR #2899) + +#### State updates should be unidirectional +Don't set the same state in multiple places. State should flow in one direction. + +> "`this.value` is set here and in `#onChange`. State updates should be unidirectional." (PFE PR #2899) + +#### Expose optional protected hooks +When subclasses may need to hook into behavior, expose an optional protected method rather than making methods protected by default. + +> "If the subclass needs to hook in here, let's expose an optional protected hook instead: `protected onSlotchange?(): void`." (PFE PR #2099) + +#### Modules don't need to wait on `load` +ES modules are always deferred, so waiting for the `load` event is unnecessary. + +> "Modules don't need to wait on `load`, since they're always deferred." (PFE PR #2563) + +#### Components should not own features that belong to external code +If a feature (like adding items to a group) belongs to consuming code, document it with demos but don't build it into the component. + +> "The idea of adding labels to the group should not be handled inside the PfLabelGroupClass. We should certainly document how to do this with demos, but those demos shouldn't be using built-in features of pf-label-group." (PFE PR #2949) + +#### Ensure hard-coded strings are internationalizable +Don't bake locale-specific strings into component logic. Consider i18n from the start. + +> "This regexp hard-codes the string 'remaining' into the component's UI. What if a downstream developer needs to put a label group on a page which is translated into french? or swahili?" (PFE PR #2949) + +#### Mark properties as private to avoid accidental API surface +Public properties are API surface that bumps the minor version. If a property doesn't need to be public, make it private. + +> "Should be private to avoid a minor release." (PFE PR #2493) + +#### Consider whether something should be CSS API or HTML attribute API +Some features (like `scrollable`) may be better expressed as CSS custom properties than HTML attributes. + +> "I wonder if `scrollable` is better defined as a css api than an html api?" (PFE PR #2099) + +### Code Style + +#### Use ECMAScript private fields +Prefer `#privateField` syntax over TypeScript `private` keyword for true encapsulation at runtime. + +> "Let's use ecmascript private fields here" (RHDS PR #436); "Prefer ecmascript private fields where possible" (PFE PR #2599) + +#### Use `override` keyword on lifecycle methods +Always use the TypeScript `override` keyword when overriding LitElement lifecycle methods. + +> RHDS PR #1149 + +#### Use `classList.toggle` with force parameter +Use `classList.toggle(name, force)` instead of if/else with add/remove. + +> Suggested `col.classList.toggle('active', index === cellIndex)` (RHDS PR #1197) + +#### Use const object literals for simple mappings +`const DIRECTION_OPPOSITES = { asc: 'desc', desc: 'asc' } as const;` instead of switch/if. + +> RHDS PR #1197 + +#### Consider TypeScript union types for constrained attribute values +Enables IDE autocompletion and catches errors at compile time. + +> "it Would be cool if we exported the union of names in each set from the icons repo. that way we could get icon name autocompletion." (RHDS PR #1732) + +#### Prefer `switch` for keyboard event handling +Use `switch` statements over if/else chains for keyboard events. + +> "Nit: prefer `switch` statements for keyboard events." (PFE PR #2570) + +#### Follow established class member ordering +Organize class members consistently: static fields, reactive properties, private fields, constructor, lifecycle methods, render, public methods, private methods. + +> "Member ordering: https://github.com/RedHat-UX/red-hat-design-system/wiki/Custom-Elements-API-Style-Guide#ordering" (PFE PR #2589) + +## Accessibility + +### Manage ARIA internally +Elements should manage their own ARIA attributes internally rather than requiring users to set them in markup. + +> "can we handle aria internally?" (RHDS PR #2449, recurring) + +### Derive accessible labels from slotted content +Automatically compute accessible labels from headline/heading slots. Always provide an accessible-label attribute escape hatch. + +> "Ok, we can adjust the code which computes the label to only consider elements slotted into the headline slot. but what if there's no headline text, we still need the accessible-label escape hatch, right?" (RHDS PR #1302) + +### Compute ARIA state privately via context +ARIA state derived from DOM structure (`aria-posinset`, `aria-setsize`) should be computed internally via context, not exposed as public attributes. + +> "We might consider making this private. The select element would provide a context." (RHDS PR #2802) + +### Use ElementInternals for ARIA semantics +Use ElementInternals to set `role`, `aria-label`, and heading levels rather than forcing users to manage them. Accept and gracefully handle common mistakes in attribute values (like `2` vs `h2`). + +> RHDS PRs #1715, #1732; "If we adopt FACE we should use `this.#internals.ariaLabelledby`." (PFE PR #2099) + +### Use ElementInternals for form association +Use the ElementInternals API for form-associated custom elements rather than wrapping or slotting native form elements. Call `setValue()` on the internals object whenever the value changes. + +> "Uses the new ElementInternals API to remove need for slotted button" (RHDS PR #550); "We'll need to attachInternals, and whenever the value changes, call `setValue()` to report back the form value." (PFE PR #2511) + +### Avoid landmark pollution +Don't put `
`, `
`, `