diff --git a/bulma-ui/src/components/Avatar.tsx b/bulma-ui/src/components/Avatar.tsx index 72e7b9979..8d763bfb1 100644 --- a/bulma-ui/src/components/Avatar.tsx +++ b/bulma-ui/src/components/Avatar.tsx @@ -161,6 +161,15 @@ export const Avatar: React.FC = ({ // 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(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(null); const { bulmaHelperClasses, rest } = useBulmaClasses(props); diff --git a/bulma-ui/src/components/Avatars.stories.tsx b/bulma-ui/src/components/Avatars.stories.tsx index 6abce945a..380e3ac6b 100644 --- a/bulma-ui/src/components/Avatars.stories.tsx +++ b/bulma-ui/src/components/Avatars.stories.tsx @@ -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 ( +
+ + {members.map(m => ( + + ))} + +
+ ); + }, +}; diff --git a/bulma-ui/src/components/Avatars.tsx b/bulma-ui/src/components/Avatars.tsx index ded3b7f63..af74cabd8 100644 --- a/bulma-ui/src/components/Avatars.tsx +++ b/bulma-ui/src/components/Avatars.tsx @@ -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. * @@ -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 @@ -78,8 +104,8 @@ export const Avatars: React.FC & { Avatar: typeof Avatar } = ({ className ); - const childArray = React.Children.toArray(children).filter( - React.isValidElement + const childArray = flattenChildren( + children ) as React.ReactElement[]; const maxCount = diff --git a/bulma-ui/src/components/__tests__/Avatar.test.tsx b/bulma-ui/src/components/__tests__/Avatar.test.tsx index 1f3838b32..7a22bb5cf 100644 --- a/bulma-ui/src/components/__tests__/Avatar.test.tsx +++ b/bulma-ui/src/components/__tests__/Avatar.test.tsx @@ -43,6 +43,27 @@ describe('Avatar', () => { ); }); + it('retries a previously failed src when switching back to it', () => { + const { rerender } = render(); + fireEvent.error(screen.getByRole('img', { hidden: true })); + expect(screen.getByText('AL')).toBeInTheDocument(); + + rerender(); + 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(); + 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(); expect(screen.getByText('CH')).toBeInTheDocument(); diff --git a/bulma-ui/src/components/__tests__/Avatars.styles.test.tsx b/bulma-ui/src/components/__tests__/Avatars.styles.test.tsx new file mode 100644 index 000000000..288c8a723 --- /dev/null +++ b/bulma-ui/src/components/__tests__/Avatars.styles.test.tsx @@ -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'); + }); +}); diff --git a/bulma-ui/src/components/__tests__/Avatars.test.tsx b/bulma-ui/src/components/__tests__/Avatars.test.tsx index fc26227f9..bbc749282 100644 --- a/bulma-ui/src/components/__tests__/Avatars.test.tsx +++ b/bulma-ui/src/components/__tests__/Avatars.test.tsx @@ -232,6 +232,82 @@ describe('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( + + <> + + + + + + + ); + 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( + + <> + + + + ); + 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( + + + <> + {['Ada Lovelace', 'Grace Hopper', 'Alan Turing'].map(n => ( + + ))} + + + ); + // 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( + + <> + + <> + + + + + + + ); + expect(screen.getByText('AL')).toBeInTheDocument(); + expect(screen.getByText('GH')).toBeInTheDocument(); + expect(screen.getByText('+2')).toBeInTheDocument(); }); it('applies the classPrefix from ConfigProvider', () => { diff --git a/bulma-ui/src/scss/components/_avatars.scss b/bulma-ui/src/scss/components/_avatars.scss index 31321cbf8..532fc64fc 100644 --- a/bulma-ui/src/scss/components/_avatars.scss +++ b/bulma-ui/src/scss/components/_avatars.scss @@ -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')}); } } @@ -59,7 +62,7 @@ $avatars-spacing-lg: 1rem !default; box-shadow: none; &:not(:first-child) { - margin-left: 0; + margin-inline-start: 0; } } } diff --git a/docs/docs/api/components/avatar.md b/docs/docs/api/components/avatar.md index a50315d86..20046052f 100644 --- a/docs/docs/api/components/avatar.md +++ b/docs/docs/api/components/avatar.md @@ -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 diff --git a/docs/docs/api/components/avatars.md b/docs/docs/api/components/avatars.md index 18a24fed3..58e377724 100644 --- a/docs/docs/api/components/avatars.md +++ b/docs/docs/api/components/avatars.md @@ -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