Fix Avatar types to follow OwnProps pattern

Make AvatarOwnProps the source of truth with inline type definitions, and have AvatarProps extend it. This follows the established pattern used by Button, Checkbox, and Alert components. Export AvatarOwnProps from the package entry point. Signed-off-by: Johan Persson <johanopersson@gmail.com>
2026-02-24 11:18:36 +01:00
parent c286160632
commit 9df65dae02
5 changed files with 140 additions and 18 deletions
@@ -0,0 +1,7 @@
+---
+'@backstage/ui': patch
+---
+
+Migrated Avatar, Checkbox, Accordion, and other components from `useStyles` to `useDefinition` hook. Exported `AvatarOwnProps` and restructured `AvatarProps` to extend it, consistent with the pattern used by other migrated components.
+
+**Affected components:** Avatar, Checkbox, Accordion
@@ -0,0 +1,103 @@
+# Implementation Plan
+
+## Status Overview
+
+- **Migrated (9):** Accordion, Alert, Avatar, Box, Button, ButtonIcon, ButtonLink, Card, Checkbox
+- **Remaining (28):** see tasks below
+- **Final cleanup:** delete useStyles + legacy types
+
+## Reference Patterns
+
+- **Simple consumer** (bg: 'consumer'): see Button definition.ts + Button.tsx
+- **Provider with utilities** (bg: 'provider'): see Box definition.ts + Box.tsx
+- **Neutral** (no bg): see Alert definition.ts + Alert.tsx
+- **Multi-sub-component:** see Accordion definition.ts (multiple defineComponent calls) + Card definition.ts
+
+## Key Migration Rules
+
+1. definition.ts: Replace `ComponentDefinition` with `defineComponent<OwnProps>()({...})`
+2. definition.ts: Move `import styles from './X.module.css'` here (remove from .tsx)
+3. definition.ts: Convert `dataAttributes` map → `propDefs` with `dataAttribute: true`
+4. definition.ts: Convert inline defaults → `default` in propDefs
+5. definition.ts: Add `children: {}`, `className: {}`, `style: {}` to propDefs
+6. .tsx: Replace `useStyles` with `useDefinition`, destructure `{ ownProps, restProps, dataAttributes }`
+7. .tsx: Use `ownProps.classes.xxx` instead of `clsx(classNames.xxx, styles.xxx)`
+8. .tsx: Remove `cleanedProps` usage → use `restProps` spread + `ownProps` destructuring
+9. .tsx: Remove `clsx` import if no longer needed
+10. .tsx: For bg providers: use `ownProps.childrenWithBgProvider` instead of manual `<BgProvider>` wrapping
+
+## bg System Mapping
+
+| Component       | bg value                                    |
+| --------------- | ------------------------------------------- |
+| Flex            | `'provider'` (currently uses useBgProvider) |
+| Grid (GridRoot) | `'provider'` (currently uses useBgProvider) |
+| Grid (GridItem) | `'provider'` (currently uses useBgProvider) |
+| All others      | none (no bg participation)                  |
+
+No unmigrated components use `useBgConsumer`.
+
+---
+
+## Tasks
+
+### Batch 1: Simple single-file components (no bg, no utility props)
+
+These are the simplest migrations — single component, no bg, no utility props.
+
+- [x] **Task 1: Migrate Avatar** — Single component, has dataAttributes for `size` and `variant`. See `Avatar/definition.ts` and `Avatar/Avatar.tsx`.
+- [x] **Task 1.5: Fix Avatar types** — `types.ts` is not according to the qa criteria. See `Avatar/types.ts`.
+- [x] **Task 2: Migrate Checkbox** — Single component, dataAttributes for `selected` and `indeterminate`. See `Checkbox/definition.ts` and `Checkbox/Checkbox.tsx`.
+- [ ] **Task 3: Migrate Container** — Single component, dataAttributes for `maxWidth`. See `Container/definition.ts` and `Container/Container.tsx`.
+- [ ] **Task 4: Migrate FieldError** — Single component, no dataAttributes. See `FieldError/definition.ts` and `FieldError/FieldError.tsx`.
+- [ ] **Task 5: Migrate FieldLabel** — Single component, dataAttributes for `size` and `disabled`. See `FieldLabel/definition.ts` and `FieldLabel/FieldLabel.tsx`.
+- [ ] **Task 6: Migrate Link** — Single component, dataAttributes for `variant`. See `Link/definition.ts` and `Link/Link.tsx`.
+- [ ] **Task 7: Migrate Skeleton** — Single component, dataAttributes for `variant`. See `Skeleton/definition.ts` and `Skeleton/Skeleton.tsx`.
+- [ ] **Task 8: Migrate Switch** — Single component, dataAttributes for `labelPosition`. See `Switch/definition.ts` and `Switch/Switch.tsx`.
+- [ ] **Task 9: Migrate Tooltip** — Single component, no dataAttributes. See `Tooltip/definition.ts` and `Tooltip/Tooltip.tsx`.
+- [ ] **Task 10: Migrate VisuallyHidden** — Single component, no dataAttributes. See `VisuallyHidden/definition.ts` and `VisuallyHidden/VisuallyHidden.tsx`.
+- [ ] **Task 11: Migrate Popover** — Single component, dataAttributes for `placement` and `triggerAction`. See `Popover/definition.ts` and `Popover/Popover.tsx`.
+- [ ] **Task 12: Migrate FullPage** — Single component, no dataAttributes. See `FullPage/definition.ts` and `FullPage/FullPage.tsx`.
+- [ ] **Task 13: Migrate ToggleButton** — Single component, dataAttributes for `size` and `variant`. See `ToggleButton/definition.ts` and `ToggleButton/ToggleButton.tsx`.
+- [ ] **Task 14: Migrate ToggleButtonGroup** — Single component, dataAttributes for `orientation`. See `ToggleButtonGroup/definition.ts` and `ToggleButtonGroup/ToggleButtonGroup.tsx`.
+
+### Batch 2: Form components (single-file, no bg, no utility props)
+
+- [ ] **Task 15: Migrate PasswordField** — Single component, dataAttributes for `size`. See `PasswordField/definition.ts` and `PasswordField/PasswordField.tsx`.
+- [ ] **Task 16: Migrate SearchField** — Single component, dataAttributes for `size`. See `SearchField/definition.ts` and `SearchField/SearchField.tsx`.
+- [ ] **Task 17: Migrate TextField** — Single component, dataAttributes for `size`. See `TextField/definition.ts` and `TextField/TextField.tsx`.
+
+### Batch 3: Components with utility props (no bg)
+
+- [ ] **Task 18: Migrate Text** — Has utilityProps. dataAttributes for `variant`, `color`, `weight`, `truncate`, `align`. See `Text/definition.ts` and `Text/Text.tsx`.
+
+### Batch 4: Components with bg: 'provider' + utility props
+
+- [ ] **Task 19: Migrate Flex** — bg provider with utilityProps. Currently manually calls useBgProvider and wraps in BgProvider. See `Flex/definition.ts` and `Flex/Flex.tsx`.
+- [ ] **Task 20: Migrate Grid** — bg provider with utilityProps. Has two sub-components: GridRoot and GridItem, both use useBgProvider. See `Grid/definition.ts` and `Grid/Grid.tsx`.
+
+### Batch 5: Multi-sub-component directories
+
+- [ ] **Task 21: Migrate Dialog** — Multiple sub-components (Dialog, DialogHeader, DialogBody, DialogFooter, DialogCloseButton). See `Dialog/definition.ts` and `Dialog/Dialog.tsx`.
+- [ ] **Task 22: Migrate HeaderPage** — Multiple sub-components (HeaderPage, HeaderPageContent). See `HeaderPage/definition.ts` and `HeaderPage/HeaderPage.tsx`.
+- [ ] **Task 23: Migrate PluginHeader** — Multiple sub-components (PluginHeader, PluginHeaderToolbar). See `PluginHeader/definition.ts` and `PluginHeader/PluginHeader.tsx` + `PluginHeaderToolbar.tsx`.
+- [ ] **Task 24: Migrate RadioGroup** — Multiple sub-components (RadioGroup, Radio). See `RadioGroup/definition.ts` and `RadioGroup/RadioGroup.tsx`.
+- [ ] **Task 25: Migrate Select** — Multiple sub-components (Select, SelectContent, SelectTrigger, SelectListBox). See `Select/definition.ts` and `Select/Select.tsx` + sub-files.
+- [ ] **Task 26: Migrate Table** — Many sub-components (TableRoot, TableHeader, TableBody, Row, Column, Cell, CellText, CellProfile). See `Table/definition.ts` and multiple .tsx files.
+- [ ] **Task 27: Migrate TablePagination** — Single component but has dataAttributes. See `TablePagination/definition.ts` and `TablePagination/TablePagination.tsx`.
+- [ ] **Task 28: Migrate Tabs** — Multiple sub-components (Tabs, TabList, Tab, TabPanel, TabsIndicators). See `Tabs/definition.ts` and `Tabs/Tabs.tsx` + `TabsIndicators.tsx`.
+- [ ] **Task 29: Migrate TagGroup** — Multiple sub-components (TagGroup, TagList, Tag). See `TagGroup/definition.ts` and `TagGroup/TagGroup.tsx`.
+- [ ] **Task 30: Migrate Menu** — Complex, multiple sub-components (Menu, MenuTrigger, MenuPopover, MenuContent, MenuItem, MenuSection, MenuSectionHeader, MenuSeparator, SubmenuTrigger). See `Menu/definition.ts` and `Menu/Menu.tsx`.
+
+### Batch 6: Final cleanup
+
+- [ ] **Task 31: Delete useStyles hook** — Remove `packages/ui/src/hooks/useStyles.ts`. Only after all components are migrated.
+- [ ] **Task 32: Remove legacy types** — Remove `ComponentDefinition`, `ClassNamesMap`, `DataAttributeValues`, `DataAttributesMap` from `packages/ui/src/types.ts`. Verify nothing else imports them.
+- [ ] **Task 33: Verify build and API reports** — Run `yarn tsc`, `yarn build:api-reports packages/ui`, check for no CSS changes, verify no changes outside packages/ui. Create changeset if API report changed.
+
+## Discoveries / Notes
+
+- 7 of 37 styled components already migrated (~19%)
+- Only 3 sub-components need bg: 'provider' (Flex, GridRoot, GridItem). No unmigrated components use useBgConsumer.
+- InternalLinkProvider is a context provider, not a styled component — no migration needed.
+- useStyles.ts already imports helpers from useDefinition (resolveResponsiveValue, processUtilityProps), confirming shared infrastructure.
@@ -281,12 +281,23 @@ export const AvatarDefinition: {
 };

 // @public (undocumented)
-export interface AvatarProps extends React.ComponentPropsWithoutRef<'div'> {
-  name: string;
-  purpose?: 'decoration' | 'informative';
-  size?: 'x-small' | 'small' | 'medium' | 'large' | 'x-large';
+export type AvatarOwnProps = {
  src: string;
-}
+  name: string;
+  size?: 'x-small' | 'small' | 'medium' | 'large' | 'x-large';
+  purpose?: 'decoration' | 'informative';
+  children?: React.ReactNode;
+  className?: string;
+  style?: React.CSSProperties;
+};
+
+// @public (undocumented)
+export interface AvatarProps
+  extends Omit<
+      React.ComponentPropsWithoutRef<'div'>,
+      'children' | 'className' | 'style'
+    >,
+    AvatarOwnProps {}

 // @public (undocumented)
 export interface BgContextValue {
@@ -16,4 +16,4 @@

 export { Avatar } from './Avatar';
 export { AvatarDefinition } from './definition';
-export type { AvatarProps } from './types';
+export type { AvatarOwnProps, AvatarProps } from './types';
@@ -16,17 +16,6 @@

 /** @public */
 export type AvatarOwnProps = {
-  size?: AvatarProps['size'];
-  purpose?: AvatarProps['purpose'];
-  src: string;
-  name: string;
-  children?: React.ReactNode;
-  className?: string;
-  style?: React.CSSProperties;
-};
-
-/** @public */
-export interface AvatarProps extends React.ComponentPropsWithoutRef<'div'> {
  /**
   * URL of the image to display
   */
@@ -50,4 +39,16 @@ export interface AvatarProps extends React.ComponentPropsWithoutRef<'div'> {
   * @defaultValue 'informative'
   */
  purpose?: 'decoration' | 'informative';
-}
+
+  children?: React.ReactNode;
+  className?: string;
+  style?: React.CSSProperties;
+};
+
+/** @public */
+export interface AvatarProps
+  extends Omit<
+      React.ComponentPropsWithoutRef<'div'>,
+      'children' | 'className' | 'style'
+    >,
+    AvatarOwnProps {}