Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions bulma-ui/src/components/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,18 @@ export const Avatar: React.FC<AvatarProps> = ({
style,
...props
}) => {
// Tracks the src that failed to load (rather than a plain boolean) so a
// new src is shown again without needing an effect to reset the flag.
// Tracks the src that failed to load. A src change clears the latch during
// render (React's "reset state when props change" pattern) so a previously
// failed src is retried when switched back to. The img is additionally
// keyed by src so a swap discards the old DOM node — a late error event
// from the previous request can't fire on a retained node and latch the
// replacement src as failed.
const [erroredSrc, setErroredSrc] = useState<string | undefined>(undefined);
const [prevSrc, setPrevSrc] = useState(src);
if (src !== prevSrc) {
setPrevSrc(src);
setErroredSrc(undefined);
}
Comment thread
allxsmith marked this conversation as resolved.
const imgRef = useRef<HTMLImageElement>(null);

const { bulmaHelperClasses, rest } = useBulmaClasses(props);
Expand Down Expand Up @@ -234,6 +243,7 @@ export const Avatar: React.FC<AvatarProps> = ({
>
{showImage && (
<img
key={src}
{...imageProps}
ref={imgRef}
src={src}
Expand Down
16 changes: 16 additions & 0 deletions bulma-ui/src/components/Avatars.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,22 @@ export const NumericSpacing: Story = {
},
};

export const Rtl: Story = {
render: function RtlExample() {
// The overlap uses margin-inline-start, so under dir="rtl" the stack
// overlaps toward the reading direction instead of spreading apart.
return (
<div dir="rtl">
<Avatars max={3} size="48x48">
{members.map(m => (
<Avatar key={m.id} name={m.name} />
))}
</Avatars>
</div>
);
},
};

export const Spaced: Story = {
render: function SpacedExample() {
return (
Expand Down
30 changes: 27 additions & 3 deletions bulma-ui/src/components/Avatars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,32 @@ export interface AvatarsProps
children?: React.ReactNode;
}

/**
* Flattens Fragment children (recursively) into a flat element array so they
* participate in max clamping and size/shape injection. Children.toArray keys
* restart at ".0" inside each fragment, so fragment children are re-keyed by
* prefixing the fragment's own key — otherwise a fragment sibling of a direct
* child would collide (both ".0") and trigger duplicate-key warnings.
*/
function flattenChildren(
children: React.ReactNode,
keyPrefix = ''
): React.ReactElement<AvatarProps>[] {
return React.Children.toArray(children).flatMap(child => {
if (!React.isValidElement(child)) return [];
if (child.type === React.Fragment) {
return flattenChildren(
(child.props as { children?: React.ReactNode }).children,
keyPrefix + child.key
);
}
const el = child as React.ReactElement<AvatarProps>;
return [
keyPrefix ? React.cloneElement(el, { key: keyPrefix + el.key }) : el,
];
});
}

/**
* Avatars component for rendering an overlapping/stacked group of `Avatar`s.
*
Expand Down Expand Up @@ -78,9 +104,7 @@ export const Avatars: React.FC<AvatarsProps> & { Avatar: typeof Avatar } = ({
className
);

const childArray = React.Children.toArray(children).filter(
React.isValidElement
) as React.ReactElement<AvatarProps>[];
const childArray = flattenChildren(children);

const maxCount =
typeof max === 'number' && Number.isInteger(max) && max >= 0
Expand Down
34 changes: 34 additions & 0 deletions bulma-ui/src/components/__tests__/Avatar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,40 @@ describe('Avatar', () => {
);
});

it('retries a previously failed src after switching away and back', () => {
const { rerender } = render(
<Avatar src="/broken.jpg" name="Ada Lovelace" />
);
const img = screen.getByRole('img', { hidden: true }) as HTMLImageElement;
fireEvent.error(img);
expect(screen.getByText('AL')).toBeInTheDocument();

rerender(<Avatar src="/fixed.jpg" name="Ada Lovelace" />);
expect(screen.getByRole('img', { name: 'Ada Lovelace' })).toHaveAttribute(
'src',
'/fixed.jpg'
);

// Switching back to the once-failed src must remount the img and retry it,
// not permanently latch onto the old failure.
rerender(<Avatar src="/broken.jpg" name="Ada Lovelace" />);
expect(screen.getByRole('img', { name: 'Ada Lovelace' })).toHaveAttribute(
'src',
'/broken.jpg'
);
});

it('remounts the img node when src changes', () => {
const { rerender } = render(<Avatar src="/a.jpg" name="Ada Lovelace" />);
const first = screen.getByRole('img', { name: 'Ada Lovelace' });
rerender(<Avatar src="/b.jpg" name="Ada Lovelace" />);
const second = screen.getByRole('img', { name: 'Ada Lovelace' });
expect(second).toHaveAttribute('src', '/b.jpg');
// A fresh DOM node (via key={src}) means a late error event from the old
// request cannot fire on a retained node and latch the new src as failed.
expect(second).not.toBe(first);
});

it('derives initials from a single-word name', () => {
render(<Avatar name="Cher" />);
expect(screen.getByText('CH')).toBeInTheDocument();
Expand Down
74 changes: 74 additions & 0 deletions bulma-ui/src/components/__tests__/Avatars.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,80 @@ describe('Avatars', () => {
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.queryByText('text node')).not.toBeInTheDocument();
});

it('counts fragment-wrapped children toward max clamping', () => {
render(
<Avatars max={2}>
<Avatar name="Ada Lovelace" />
<>
<Avatar name="Grace Hopper" />
<Avatar name="Katherine Johnson" />
<Avatar name="Margaret Hamilton" />
</>
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('GH')).toBeInTheDocument();
expect(screen.queryByText('KJ')).not.toBeInTheDocument();
expect(screen.getByText('+2')).toBeInTheDocument();
});

it('injects the group size and shape into fragment-wrapped children', () => {
render(
<Avatars size="64x64" shape="square">
<>
<Avatar name="Ada Lovelace" data-testid="wrapped" />
</>
</Avatars>
);
const wrapped = screen.getByTestId('wrapped');
expect(wrapped).toHaveClass('is-64x64');
expect(wrapped).toHaveClass('is-square');
});

it('flattens nested fragments and ignores non-elements inside them', () => {
render(
<Avatars max={2}>
<>
<Avatar name="Ada Lovelace" />
{'stray text'}
<>
<Avatar name="Grace Hopper" />
<Avatar name="Katherine Johnson" />
<Avatar name="Margaret Hamilton" />
</>
</>
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('GH')).toBeInTheDocument();
expect(screen.getByText('+2')).toBeInTheDocument();
expect(screen.queryByText('stray text')).not.toBeInTheDocument();
});

it('renders mixed direct and fragment children without duplicate-key warnings', () => {
const consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
try {
render(
<Avatars>
<Avatar name="Ada Lovelace" />
<>
<Avatar name="Grace Hopper" />
<Avatar name="Katherine Johnson" />
</>
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('GH')).toBeInTheDocument();
expect(screen.getByText('KJ')).toBeInTheDocument();
expect(consoleError).not.toHaveBeenCalled();
} finally {
consoleError.mockRestore();
}
});

it('applies the classPrefix from ConfigProvider', () => {
Expand Down
6 changes: 4 additions & 2 deletions bulma-ui/src/scss/components/_avatars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ $avatars-spacing-lg: 1rem !default;
cv.getVar('avatars-ring-color');

&:not(:first-child) {
margin-left: calc(-1 * #{cv.getVar('avatars-spacing')});
// Logical property so the stack overlaps toward the reading direction
// (a physical margin-left pulls avatars apart under dir="rtl").
margin-inline-start: calc(-1 * #{cv.getVar('avatars-spacing')});
}
}

Expand All @@ -59,7 +61,7 @@ $avatars-spacing-lg: 1rem !default;
box-shadow: none;

&:not(:first-child) {
margin-left: 0;
margin-inline-start: 0;
}
}
}
Expand Down
Loading