[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

未关闭
#36,684 0 条评论 0 个 reaction 已指派 1 人 在 GitHub 查看

维护者通常 2 天内回复

@mainframev 已经在做这个了。

开始于 2026年9月19日。

评估

这个 Issue 还没有评估数据。

描述

Component: TeachingPopover Headless Components Headless Hooks
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: #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. Omission — layout 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

主要语言
TypeScript
星标
20.4k
派生
3k
平均合并
3 天 21 小时
30 天内合并 PR
99

环境准备

在 Codespaces 中打开

在浏览器里用你自己的 GitHub 账号启动这个项目的开发容器。

  • 没有 Dockerfile 或 Docker Compose 文件
  • 有 Pull Request 模板
  • 没有贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

microsoft/fluentui 的其他 Issue

查看 microsoft/fluentui 的全部 Issue

相似的 Issue

更多 TypeScript Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。