Skip to content
Closed
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
9 changes: 9 additions & 0 deletions bulma-ui/src/components/Avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,15 @@ export const Avatar: React.FC<AvatarProps> = ({
// 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.
const [erroredSrc, setErroredSrc] = useState<string | undefined>(undefined);
// Forget the failure whenever `src` changes (render-time state adjustment):
// without this, switching away and back to a previously failed URL would
// stay latched to the fallback forever — the error is per-mount history,
// not per-URL truth (a transient failure may well succeed on retry).
const [prevSrc, setPrevSrc] = useState(src);
if (src !== prevSrc) {
setPrevSrc(src);
setErroredSrc(undefined);
}
const imgRef = useRef<HTMLImageElement>(null);

const { bulmaHelperClasses, rest } = useBulmaClasses(props);
Expand Down
22 changes: 22 additions & 0 deletions bulma-ui/src/components/Avatars.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,25 @@ export const Spaced: Story = {
);
},
};

export const RTLOverlap: Story = {
parameters: {
docs: {
description: {
story:
'The overlap uses a logical margin (`margin-inline-start`), so under `dir="rtl"` the stack still overlaps instead of the avatars being pulled apart.',
},
},
},
render: function RTLOverlapExample() {
return (
<div dir="rtl">
<Avatars max={4}>
{members.map(m => (
<Avatar key={m.id} name={m.name} />
))}
</Avatars>
</div>
);
},
};
32 changes: 29 additions & 3 deletions bulma-ui/src/components/Avatars.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,32 @@ import { Avatar, AvatarProps } from './Avatar';
/** Valid overlap spacing values for the Avatars component. */
export type AvatarsSpacing = 'sm' | 'md' | 'lg';

// React.Children.toArray does not descend into Fragments, so a fragment
// wrapper (natural when interleaving a static avatar with a mapped list)
// would count as ONE child: `max` never clamps, and the group's size/shape
// would be cloned onto the fragment and silently dropped. Flatten fragments
// (recursively) before counting. Flattened elements are re-keyed with their
// fragment's key as a prefix so children hoisted from different levels can
// never collide with same-position siblings.
const flattenChildren = (
nodes: React.ReactNode,
keyPrefix = ''
): React.ReactElement[] =>
React.Children.toArray(nodes).flatMap(node => {
if (!React.isValidElement(node)) return [];
if (node.type === React.Fragment) {
return flattenChildren(
(node.props as { children?: React.ReactNode }).children,
`${keyPrefix}${node.key ?? ''}`
);
}
return [
keyPrefix
? React.cloneElement(node, { key: `${keyPrefix}${node.key}` })
: node,
];
});

/**
* Props for the Avatars component.
*
Expand All @@ -15,7 +41,7 @@ export type AvatarsSpacing = 'sm' | 'md' | 'lg';
* @property {AvatarProps['shape']} [shape] - Uniform shape applied to every child `Avatar` (and the surplus avatar).
* @property {AvatarsSpacing | number} [spacing] - Space between avatars: a `'sm'`/`'md'`/`'lg'` preset or a pixel `number`. Default `'md'`.
* @property {boolean} [spaced] - Lay the avatars out side by side (non-overlapping) with `spacing` as the gap. Default `false`.
* @property {React.ReactNode} [children] - `Avatar` elements to render inside the group.
* @property {React.ReactNode} [children] - `Avatar` elements to render inside the group. Fragments are flattened, so `max` counting and the group `size`/`shape` work through them.
*/
export interface AvatarsProps
extends
Expand Down Expand Up @@ -78,8 +104,8 @@ export const Avatars: React.FC<AvatarsProps> & { Avatar: typeof Avatar } = ({
className
);

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

const maxCount =
Expand Down
21 changes: 21 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,27 @@ describe('Avatar', () => {
);
});

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

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

// Back to the URL that failed once — must retry (mount the img again),
// not stay latched to initials: the failure may have been transient.
rerender(<Avatar src="/a.jpg" name="Ada Lovelace" />);
expect(screen.getByRole('img', { name: 'Ada Lovelace' })).toHaveAttribute(
'src',
'/a.jpg'
);
expect(screen.queryByText('AL')).not.toBeInTheDocument();
});

it('derives initials from a single-word name', () => {
render(<Avatar name="Cher" />);
expect(screen.getByText('CH')).toBeInTheDocument();
Expand Down
46 changes: 46 additions & 0 deletions bulma-ui/src/components/__tests__/Avatars.styles.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// The overlap direction bug (issue #265) is invisible to jsdom's className
// assertions — jsdom doesn't lay out CSS, and a physical margin-left looks
// identical to a logical margin-inline-start until a dir="rtl" ancestor flips
// the inline axis. Compile the real SCSS partial and assert the rules use
// logical properties (same approach as Badge.styles.test.tsx).
import * as sass from 'sass';
import path from 'path';

let compiledCss: string;

beforeAll(() => {
const result = sass.compile(
path.resolve(__dirname, '../../scss/components/_avatars.scss'),
{
loadPaths: [path.resolve(__dirname, '../../../../node_modules')],
quietDeps: true,
logger: sass.Logger.silent,
}
);
compiledCss = result.css.replace(/\s+/g, ' ');
});

describe('Avatars overlap styles', () => {
it('overlaps via margin-inline-start so RTL stacks still overlap', () => {
const overlapRule = compiledCss.match(
/\.avatars \.avatar:not\(:first-child\)\s*\{([^}]*)\}/
);
expect(overlapRule).not.toBeNull();
expect(overlapRule?.[1]).toContain('margin-inline-start: calc(');
expect(overlapRule?.[1]).not.toContain('margin-left');
});

it('resets the spaced layout via the same logical property', () => {
const spacedRule = compiledCss.match(
/\.avatars\.is-spaced \.avatar:not\(:first-child\)\s*\{([^}]*)\}/
);
expect(spacedRule).not.toBeNull();
expect(spacedRule?.[1]).toContain('margin-inline-start: 0');
expect(spacedRule?.[1]).not.toContain('margin-left');
});

it('emits no physical inline-direction margins anywhere in the partial', () => {
expect(compiledCss).not.toContain('margin-left');
expect(compiledCss).not.toContain('margin-right');
});
});
76 changes: 76 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,82 @@ describe('Avatars', () => {
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
// The text node must actually be dropped, not just coexist with the
// avatar — without this assertion the test passes with or without the
// isValidElement filter.
expect(screen.queryByText('text node')).not.toBeInTheDocument();
});

it('counts Fragment children individually for max', () => {
render(
<Avatars max={2}>
<>
<Avatar name="Ada Lovelace" />
<Avatar name="Grace Hopper" />
<Avatar name="Alan Turing" />
<Avatar name="Katherine Johnson" />
</>
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('GH')).toBeInTheDocument();
expect(screen.getByText('+2')).toBeInTheDocument();
expect(screen.queryByText('AT')).not.toBeInTheDocument();
expect(screen.queryByText('KJ')).not.toBeInTheDocument();
});

it('injects the group size through Fragment children', () => {
const { container } = render(
<Avatars size="48x48">
<>
<Avatar name="Ada Lovelace" />
</>
</Avatars>
);
expect(container.querySelector('.avatar')).toHaveClass('is-48x48');
});

it('flattens a keyed mapped list inside a Fragment next to a sibling without key collisions', () => {
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
render(
<Avatars max={2}>
<Avatar name="Zz Static" />
<>
{['Ada Lovelace', 'Grace Hopper', 'Alan Turing'].map(n => (
<Avatar key={n} name={n} />
))}
</>
</Avatars>
);
// 4 children, max 2 → 2 visible + "+2" surplus.
expect(screen.getByText('ZS')).toBeInTheDocument();
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('+2')).toBeInTheDocument();
// Re-keying flattened fragment children must not produce React's
// duplicate-key warning.
const keyWarnings = errorSpy.mock.calls.filter(args =>
String(args[0]).includes('same key')
);
expect(keyWarnings).toHaveLength(0);
errorSpy.mockRestore();
});

it('flattens nested Fragments', () => {
render(
<Avatars max={2}>
<>
<Avatar name="Ada Lovelace" />
<>
<Avatar name="Grace Hopper" />
<Avatar name="Alan Turing" />
<Avatar name="Katherine Johnson" />
</>
</>
</Avatars>
);
expect(screen.getByText('AL')).toBeInTheDocument();
expect(screen.getByText('GH')).toBeInTheDocument();
expect(screen.getByText('+2')).toBeInTheDocument();
});

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

// Logical property, not margin-left: under dir="rtl" the stack grows
// right-to-left, and a physical negative left margin would pull the
// avatars apart instead of overlapping them.
&:not(:first-child) {
margin-left: calc(-1 * #{cv.getVar('avatars-spacing')});
margin-inline-start: calc(-1 * #{cv.getVar('avatars-spacing')});
}
}

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

&:not(:first-child) {
margin-left: 0;
margin-inline-start: 0;
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion docs/docs/api/components/avatar.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ import { Avatar } from '@allxsmith/bestax-bulma';
### Photo with Automatic Fallback

A working photo renders as an image; if the `src` fails to load, `Avatar` swaps to initials
derived from `name` automatically — no broken-image icon.
derived from `name` automatically — no broken-image icon. The failure is remembered per URL
only while `src` is unchanged: changing `src` (including back to a URL that failed before)
retries the load.

```tsx live
<Avatars spaced>
Expand Down
6 changes: 5 additions & 1 deletion docs/docs/api/components/avatars.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,16 @@ import { Avatars, Avatar } from '@allxsmith/bestax-bulma';
| `spacing` | `'sm' \| 'md' \| 'lg' \| number` | `'md'` | Space between avatars: a preset or a pixel `number`. The overlap distance, or the gap when `spaced`. |
| `spaced` | `boolean` | `false` | Lay the avatars out side by side (non-overlapping), using `spacing` as the gap. |
| `className` | `string` | — | Additional CSS classes. |
| `children` | `React.ReactNode` | — | `Avatar` elements to render inside the group. |
| `children` | `React.ReactNode` | — | `Avatar` elements to render inside the group. Fragments are flattened, so `max` counting and the group `size`/`shape` work through them. |
| ... | All standard HTML and Bulma helper props | | (See [Helper Props](../helpers/usebulmaclasses)) |

`Avatar` is also attached as a compound static (`Avatars.Avatar`), so you can import just the
container.

Fragments count individually: wrapping children in `<>…</>` — natural when interleaving a
static avatar with a mapped list — behaves exactly like passing them directly. The overlap
also uses logical CSS margins, so right-to-left layouts (`dir="rtl"`) stack correctly.

---

## Usage
Expand Down
Loading