-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add tooltip support to Avatar, AvatarGroup, and WorkspaceAvatar #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
0da6e69
5cb7993
83b828b
50fb3ab
4323bc8
835f7de
7c3ebf4
df5147d
8c14a05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { Avatar } from "@makeplane/propel/components/avatar"; | ||
| import { AvatarGroup } from "@makeplane/propel/components/avatar-group"; | ||
|
|
||
| export default function WithTooltipsDemo() { | ||
| return ( | ||
| <AvatarGroup size="sm"> | ||
| <Avatar alt="Ada Lovelace" tooltip fallback="AL" src="https://i.pravatar.cc/64?img=47" /> | ||
| <Avatar alt="Grace Hopper" tooltip fallback="GH" src="https://i.pravatar.cc/64?img=32" /> | ||
| <Avatar alt="Linus Torvalds" tooltip fallback="LT" /> | ||
| <Avatar alt="4 more members" tooltip fallback="+4" /> | ||
| </AvatarGroup> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { Avatar } from "@makeplane/propel/components/avatar"; | ||
|
|
||
| export default function WithTooltipDemo() { | ||
| return ( | ||
| <Avatar | ||
| size="md" | ||
| alt="Ada Lovelace" | ||
| tooltip | ||
| fallback="AL" | ||
| src="https://i.pravatar.cc/128?img=47" | ||
| /> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import { WorkspaceAvatar } from "@makeplane/propel/components/workspace-avatar"; | ||
|
|
||
| export default function WithTooltipDemo() { | ||
| return ( | ||
| <WorkspaceAvatar | ||
| size="md" | ||
| alt="Plane workspace" | ||
| tooltip | ||
| fallback="PV" | ||
| src="https://avatars.githubusercontent.com/u/73642778?s=128" | ||
| /> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { | |
| getAvatarVariantSeed, | ||
| } from "../../elements/avatar"; | ||
| import { Icon } from "../../internal/icon"; | ||
| import { Tooltip } from "../tooltip/tooltip"; | ||
| import { AvatarGroupContext } from "./avatar-group-context"; | ||
|
|
||
| export type AvatarProps = Omit<AvatarElementProps, "size"> & { | ||
|
|
@@ -29,6 +30,11 @@ export type AvatarProps = Omit<AvatarElementProps, "size"> & { | |
| fallback?: React.ReactNode; | ||
| /** Milliseconds before the fallback shows, to avoid a flash while `src` loads quickly. */ | ||
| delay?: number; | ||
| /** | ||
| * Name shown in a tooltip on hover. Pass `true` to reuse `alt`, or a string to override it. Omit | ||
| * for no tooltip. | ||
| */ | ||
| tooltip?: boolean | string; | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -37,7 +43,7 @@ export type AvatarProps = Omit<AvatarElementProps, "size"> & { | |
| * Pass `src` for the photo and `fallback` for initials; the initials color is chosen automatically | ||
| * and is not a consumer prop. | ||
| */ | ||
| export function Avatar({ size, src, alt, fallback, delay, ...props }: AvatarProps) { | ||
| export function Avatar({ size, src, alt, fallback, delay, tooltip, ...props }: AvatarProps) { | ||
| // Base UI shows the fallback whenever the image is absent, loading, or failed, so the | ||
| // colored-initials styling lives on the Fallback element itself. The anonymous person icon | ||
| // renders in the icon slot over the root's neutral backdrop when there are no initials. | ||
|
|
@@ -54,11 +60,21 @@ export function Avatar({ size, src, alt, fallback, delay, ...props }: AvatarProp | |
| // skipped rather than announced as a nameless image (the name lives in adjacent text). This is the | ||
| // only correct pair — a `role="img"` with no name is an axe violation. | ||
| const a11y = alt != null ? { role: "img", "aria-label": alt } : { "aria-hidden": true }; | ||
| return ( | ||
| // `true` reuses `alt`; a non-empty string overrides it. Empty string / missing `alt` → no tip — | ||
| // an avatar with no `alt` is `aria-hidden`, so it must not show a hover-only tooltip either. | ||
| const tooltipLabel = alt == null ? undefined : tooltip === true ? alt : tooltip || undefined; | ||
| const avatar = ( | ||
| // Base UI `Avatar` behavior/context grafts onto the styled `elements/avatar` parts via `render` | ||
| // (behavior part outer). `{...props}` spreads before the hardcoded a11y attrs so a stray | ||
| // same-named prop can never silently override them (matches `components/workspace-avatar`). | ||
| <BaseAvatar.Root {...props} render={<AvatarElement size={effectiveSize} />} {...a11y}> | ||
| // `tabIndex` only when a tooltip is actually attached — a Tooltip's trigger must be focusable | ||
| // for its documented focus-opens contract to hold; a plain avatar stays out of tab order. | ||
| <BaseAvatar.Root | ||
| {...props} | ||
| render={<AvatarElement size={effectiveSize} />} | ||
| {...a11y} | ||
| tabIndex={tooltipLabel ? 0 : undefined} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This overrides a consumer-provided tabIndex, even when no tooltip is shown.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in df5147d — consumer |
||
| > | ||
| {src ? <BaseAvatar.Image render={<AvatarImage />} src={src} alt="" /> : null} | ||
| {hasInitials ? ( | ||
| <BaseAvatar.Fallback delay={delay} render={<AvatarFallback variant={resolvedVariant} />}> | ||
|
|
@@ -71,4 +87,5 @@ export function Avatar({ size, src, alt, fallback, delay, ...props }: AvatarProp | |
| )} | ||
| </BaseAvatar.Root> | ||
| ); | ||
| return tooltipLabel ? <Tooltip label={tooltipLabel}>{avatar}</Tooltip> : avatar; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A string tooltip is ignored when alt is missing, contrary to the documented behavior.
suggestion: Support the string independently or document that alt is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 8c14a05 — a string
tooltipworks withoutaltand becomes the accessible name.