Skip to content

[Bug]: headless TeachingPopoverCarouselFooter drops the layout prop and the slot order it drives, so the offset footer's DOM and tab order cannot be reproduced through its renderer #36684

Description

Component

TeachingPopover

Package version

@fluentui/react-headless-components-preview 0.2.5 (repo master); the reference behaviour is
@fluentui/react-teaching-popover 9.7.5.

React version

19.2.0

Environment

node v22.12.0, TypeScript as pinned by the repo root
@fluentui/react-headless-components-preview 0.2.5
@fluentui/react-teaching-popover 9.7.5

Current Behavior

@fluentui/react-teaching-popover's carousel footer has a layout prop ('centered' | 'offset',
declared at TeachingPopoverCarouselFooter.types.ts:22,35, defaulted to 'centered' at
useTeachingPopoverCarouselFooter.ts:14, carried on state at .types.ts:51-52), and its renderer
places the previous slot on a different side of the children depending on it —
packages/react-components/react-teaching-popover/library/src/components/TeachingPopoverCarouselFooter/renderTeachingPopoverCarouselFooter.tsx:14-22:

const { layout } = state;

return (
  <state.root>
    {layout === 'centered' && state.previous && <state.previous />}
    {state.root.children}
    {layout === 'offset' && state.previous && <state.previous />}
    <state.next />
  </state.root>
);

centered renders (previous, children, next); offset renders (children, previous, next). The
offset styles depend on that order: useTeachingPopoverCarouselFooterStyles.styles.ts:26-31 pushes
the buttons to the end with '& :first-child': { marginInlineEnd: 'auto' }, which lands on the
page-count/nav children only because they come first in the DOM in that mode.

The headless copy keeps neither the prop nor the branch.
packages/react-components/react-headless-components-preview/library/src/components/TeachingPopover/TeachingPopoverCarouselFooter/renderTeachingPopoverCarouselFooter.tsx:13-19:

return (
  <state.root>
    {state.previous && <state.previous />}
    {state.root.children}
    <state.next />
  </state.root>
);

useTeachingPopoverCarouselFooter.ts:9-28 reads no layout and returns none, and
TeachingPopoverCarouselFooter.types.ts:23,25 reduce props and state to
ComponentProps<…Slots> / ComponentState<…Slots>. The order is fixed at
(previous, children, next).

Mounting both packages' footers in jsdom with the same children (<span>1 of 3</span> between
Back and Next; class attributes stripped from the output):

--- griffel, layout omitted (default centered)
<div><button>Back</button><span>1 of 3</span><button>Next</button></div>
--- griffel, layout="offset"
<div><span>1 of 3</span><button>Back</button><button>Next</button></div>
--- headless, layout omitted
<div><button>Back</button><span>1 of 3</span><button>Next</button></div>
--- headless, layout="offset"
<div><button>Back</button><span>1 of 3</span><button>Next</button></div>

(layout is not on the headless props type, and in the last case the attribute does not reach the
root at runtime either.)

The consequence is that the offset arrangement can be recreated visually over the headless footer
(CSS order, or margins on the previous slot) but not structurally. DOM order determines
sequential focus order and the accessibility tree's reading order, and CSS order changes neither
(css-flexbox-1 §5.4: order "does not affect ordering in non-visual media" nor "the default
traversal order of sequential navigation modes"). So any footer built on the headless hook +
renderer tabs and reads Back → [children] → Next, while the reference's offset footer tabs and
reads [children] → Back → Next. Reproducing the latter means bypassing
renderTeachingPopoverCarouselFooter — re-implementing the render, or folding previous into
root.children before calling it.

Expected Behavior

Either resolution is coherent, and the main ask here is to learn which one is intended:

  1. Intentional narrowing — headless ships one order, and layout is a styling-layer concern.
    If so it would help to have that stated somewhere: feat(react-headless-components-preview): add TeachingPopover  #36205 lists appearance, trapFocus, and
    inline as the props intentionally dropped from the v9 component and does not mention layout,
    and it describes this footer as returning only { root }, which the merged hook (returning
    previous and next too) already goes beyond — so from outside it is hard to tell whether
    layout was weighed or fell out with the styles.

  2. Omissionlayout stays on the headless props/state (default 'centered'), the renderer
    keeps the layout-dependent order, and the hook stamps data-layout on the root, the way the same
    package already handles the other layout props it kept (AvatarGroup/useAvatarGroup.ts:16,
    SwatchPicker/useSwatchPicker.ts:18, MessageBar/useMessageBar.ts:17,
    MessageBar/MessageBarActions/useMessageBarActions.ts:20; typed on the root slot as at
    AvatarGroup.types.ts:12). A prop that changes DOM order is structure rather than skin, which is
    the part a headless layer is expected to keep; the two looks then become CSS on [data-layout].

Reproduction

import * as React from 'react';
import { TeachingPopoverCarouselFooter } from '@fluentui/react-headless-components-preview/teaching-popover';

// Renders <div><button>Back</button><span>1 of 3</span><button>Next</button></div>.
// No prop yields <div><span>1 of 3</span><button>Back</button><button>Next</button></div>,
// which is what @fluentui/react-teaching-popover renders for layout="offset".
<TeachingPopoverCarouselFooter previous="Back" next="Next">
  <span>1 of 3</span>
</TeachingPopoverCarouselFooter>;

Steps to reproduce

  1. Render the snippet above (or mount both packages' footers side by side in jsdom, as the output
    block under Current Behavior does).
  2. Inspect the root's children: the previous button is always first.
  3. Compare with @fluentui/react-teaching-popover's <TeachingPopoverCarouselFooter layout="offset">,
    whose previous button follows the children.

Are you reporting an Accessibility issue?

no — this is a question about the headless API surface. The tab/reading-order difference is what the
missing prop takes with it, not a screen-reader behaviour claim.

Discovery context

Found while building a styling layer over @fluentui/react-headless-components-preview that
reproduces @fluentui/react-teaching-popover's look and props (PR #36656). Restoring layout there
was straightforward for the visual half; getting the offset DOM order required composing the
previous slot into root.children before calling the headless renderer, i.e. working around the
render rather than using it. That layer is moving to the headless order as-is (offset reproduced
visually) and records the tab/reading-order difference from @fluentui/react-teaching-popover as a
known delta pending this question, so nothing in that PR depends on a particular answer.

Proposed fix

Only if (2) is the answer. Keep the prop where the reference keeps it and stamp it for CSS:

// TeachingPopoverCarouselFooter.types.ts
export type TeachingPopoverCarouselFooterLayout = 'offset' | 'centered';

export type TeachingPopoverCarouselFooterProps = ComponentProps<TeachingPopoverCarouselFooterSlots> & {
  layout?: TeachingPopoverCarouselFooterLayout;
};

export type TeachingPopoverCarouselFooterState = ComponentState<TeachingPopoverCarouselFooterSlots> &
  Required<Pick<TeachingPopoverCarouselFooterProps, 'layout'>>;
// useTeachingPopoverCarouselFooter.ts
const { layout = 'centered', ...rest } = props;
const root = slot.always(getIntrinsicElementProps('div', { ref, ...rest }), { elementType: 'div' });
root['data-layout'] = layout;
return { layout, components, root, previous, next };
// renderTeachingPopoverCarouselFooter.tsx — the reference's branch, verbatim
{state.layout === 'centered' && state.previous && <state.previous />}
{state.root.children}
{state.layout === 'offset' && state.previous && <state.previous />}
<state.next />

Additive: with layout omitted the rendered order is unchanged, so existing consumers see no
difference beyond a data-layout="centered" attribute on the root. Happy to open the PR, with a
DOM-order test for both values, if (2) is the direction.

Suggested severity

Medium - Has workaround (own the render: fold previous into root.children, or skip
renderTeachingPopoverCarouselFooter and render the slots directly — both give up the renderer the
package exports for exactly this).

Are you willing to submit a PR to fix?

yes

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions