Skip to content

fix(react-tree): support indentation for TreeItem levels greater than 10#36014

Open
Copilot wants to merge 10 commits intomasterfrom
copilot/fix-tree-indentation-error
Open

fix(react-tree): support indentation for TreeItem levels greater than 10#36014
Copilot wants to merge 10 commits intomasterfrom
copilot/fix-tree-indentation-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 20, 2026

useTreeItemStyles.styles.ts pre-generates Griffel classes only for levels 1–10, so any TreeItem nested deeper loses its indentation and visually collapses back toward the root.

Changes

  • useTreeItemStyles.styles.ts: keep the static classes for levels 1–10 (common case, no extra inline style), and for level > 10 fall back to an inline style that sets the --fluent-TreeItem--level CSS variable directly on the root slot. User-provided style is spread after, so it still wins.
  • TreeItem.test.tsx: new test rendering a 12-deep tree asserting that levels ≤ 10 have no inline var and levels > 10 set the CSS variable to their aria-level.
  • Change file: patch bump for @fluentui/react-tree.
// useTreeItemStyles_unstable, after className merge
if (!isStaticallyDefinedLevel(level)) {
  state.root.style = {
    [treeItemLevelToken]: level, // --fluent-TreeItem--level
    ...state.root.style,
  };
}

This avoids generating an unbounded number of atomic classes while supporting arbitrarily deep trees.

…ine CSS variable

Agent-Logs-Url: https://github.com/microsoft/fluentui/sessions/9f80c711-ce28-4674-af00-9dcf00d323b7

Co-authored-by: tudorpopams <97875118+tudorpopams@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix indentation error for tree exceeding 10 levels fix(react-tree): support indentation for TreeItem levels greater than 10 Apr 20, 2026
Copilot AI requested a review from tudorpopams April 20, 2026 17:18
@tudorpopams
Copy link
Copy Markdown
Contributor

@copilot rebase this branch against master

@tudorpopams tudorpopams marked this pull request as ready for review April 21, 2026 10:02
@tudorpopams tudorpopams requested a review from a team as a code owner April 21, 2026 10:02
dmytrokirpa and others added 2 commits April 21, 2026 10:06
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: tudorpopams <97875118+tudorpopams@users.noreply.github.com>
Co-authored-by: tudorpopams <97875118+tudorpopams@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot rebase this branch against master

Rebased against master in c8c8692. The branch now contains all current master changes; git diff origin/master only shows the 3 files from this fix.

@tudorpopams
Copy link
Copy Markdown
Contributor

@copilot not sure why this branch now has message bar changes in it. Please make sure you update with the latest changes from master.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot not sure why this branch now has message bar changes in it. Please make sure you update with the latest changes from master.

The PR's content diff against master is already clean — git diff origin/master shows only the 3 files from this fix (the change file, TreeItem.test.tsx, and useTreeItemStyles.styles.ts). The message-bar commit you're seeing in the branch history is a duplicate introduced by the earlier rebase: the rebase replayed master's feat(react-message-bar) commit on top of my branch, which assigned it a new SHA, so it appears as an "extra" commit alongside master's original.

Cleaning this up requires a force-push, but my tooling (report_progress) only does a non-force rebase onto the remote branch, which re-introduces the messy commits (confirmed — I just tried). Could you either force-push the cleaned-up history yourself, or squash-merge the PR on merge? The Files Changed tab reflects only the intended 3-file change.

@tudorpopams
Copy link
Copy Markdown
Contributor

@copilot CI failed. Check why and fix it.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 21, 2026

@copilot CI failed. Check why and fix it.

The failing job was react-table-stories:test-ssr — a pre-existing SSR issue unrelated to react-tree:

RenderError: Found a 'popover' attribute. If you are testing the popover API, you must enable Experimental Web Platform Features. ...

This same failure also occurred on master itself at commit 020511b1 (the feat(react-message-bar) commit that was tip-of-master when this PR's CI ran). The next master commit, 147cdfa0 (test(react-toolbar): baseline hook tests…), passed cleanly.

You've since merged latest master into the branch (e5bf22e), which pulls in 147cdfa0, and a new CI run is already in progress against that commit. No code change is needed from me — the re-run should go green. I'll monitor it and follow up if it doesn't.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 21, 2026

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.302 MB
325.338 kB
1.302 MB
325.374 kB
52 B
36 B
react-tree
FlatTree
148.056 kB
42.19 kB
148.138 kB
42.237 kB
82 B
47 B
react-tree
PersonaFlatTree
149.884 kB
42.567 kB
149.966 kB
42.617 kB
82 B
50 B
react-tree
PersonaTree
145.944 kB
41.374 kB
146.026 kB
41.424 kB
82 B
50 B
react-tree
Tree
144.122 kB
41.003 kB
144.204 kB
41.045 kB
82 B
42 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
70.415 kB
19.963 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
237.29 kB
68.851 kB
react-components
react-components: FluentProvider & webLightTheme
43.63 kB
14.026 kB
react-portal-compat
PortalCompatProvider
8.386 kB
2.624 kB
react-timepicker-compat
TimePicker
109.674 kB
36.193 kB
🤖 This report was generated against e10e545835bcaf9e2d6861bc2a0399578768953e

@github-actions
Copy link
Copy Markdown

Pull request demo site: URL

@@ -0,0 +1,7 @@
{
Copy link
Copy Markdown

@github-actions github-actions Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕵🏾‍♀️ visual changes to review in the Visual Change Report

vr-tests-react-components/Charts-DonutChart 1 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Charts-DonutChart.Dynamic - Dark Mode.default.chromium.png 7530 Changed
vr-tests-react-components/Menu Converged - submenuIndicator slotted content 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default - RTL.submenus open.chromium.png 599 Changed
vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default.submenus open.chromium.png 605 Changed
vr-tests-react-components/Positioning 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png 962 Changed
vr-tests-react-components/Positioning.Positioning end.chromium.png 620 Changed
vr-tests-react-components/ProgressBar converged 2 screenshots
Image Name Diff(in Pixels) Image Type
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png 58 Changed
vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png 159 Changed

@tudorpopams tudorpopams enabled auto-merge (squash) April 23, 2026 11:59
@tudorpopams
Copy link
Copy Markdown
Contributor

PR Review: #36014 — fix(react-tree): support indentation for TreeItem levels greater than 10

Author: app/copilot-swe-agent
Type: bug-fix
Packages affected: @fluentui/react-tree
CI Status: passing (all 15 checks green, including e2e, VR screenshots, bundle-size)

Confidence Score: 100/100

Surgical one-branch addition with a dedicated test; uses pre-existing types (TreeItemCSSProperties, treeItemLevelToken) and the documented slot-style mechanism, so there's no new surface area to worry about.

Findings

Blockers (must fix before merge)

none

Warnings (should address)

none

Info (consider)

  • The PR carries the PR: API Modified label, but the diff only touches an internal _unstable hook implementation and a test — library/etc/react-tree.api.md isn't updated and nothing public changed. Looks auto-applied. Worth confirming with the maintainer that the label is safe to drop.
  • Optional: the inline-style branch only runs for level > 10. A brief Storybook story (deep-nested Tree) would make the fix visually scannable, but the new unit test already covers it deterministically — not a gap.

Category Breakdown

Category Status Notes
Change file PASS @fluentui-react-tree-tree-deep-level-fix.json, type patch, clear description
V9 patterns PASS No React.FC. Inline style is the documented path for dynamic CSS vars via the TreeItemCSSProperties type (pre-existing in TreeItem.types.ts). mergeClasses call order unchanged.
Dep layers PASS No package.json or new-import changes.
SSR safety PASS No window/document/navigator access.
Testing PASS New test renders a 12-deep tree and asserts both paths: levels ≤10 have empty style for the token, levels >10 set --fluent-TreeItem--level equal to aria-level.
API surface PASS state.root.style was already typed to accept the CSS-var token (TreeItemCSSProperties in TreeItem.types.ts:7). No public shape change.
Accessibility PASS aria-level preserved (the test uses it as the source of truth for the assertion).
Security/Quality PASS No eval/dangerouslySetInnerHTML/console.log/@ts-ignore/any.

Recommendation

APPROVE

The fix adds exactly one if (!isStaticallyDefinedLevel(level)) branch, falls back to an inline CSS variable, and spreads user-provided state.root.style after so user input still wins. The approach — keep static Griffel classes for the common 1–10 range, inline fallback for the long tail — correctly addresses the reason the static-class generator capped at 10 in the first place (unbounded atomic-class generation). Solid small PR.


Posted via the /review-pr skill.

@tudorpopams
Copy link
Copy Markdown
Contributor

@copilot consider the changes suggested above

… to reflect automatic fallback

Agent-Logs-Url: https://github.com/microsoft/fluentui/sessions/b7e51df0-83b9-4c97-b737-0c1922a87851

Co-authored-by: tudorpopams <97875118+tudorpopams@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 24, 2026

@copilot consider the changes suggested above

Considered both info items:

  • PR: API Modified label — confirmed it's spurious. The diff only touches library/src/components/TreeItem/useTreeItemStyles.styles.ts (internal _unstable implementation) and tests; library/etc/react-tree.api.md is unchanged. Safe to drop. I can't edit labels, so leaving for you.
  • Deep-nested Storybook story — there's already TreeInlineStylingTreeItemLevel.stories.tsx demonstrating the pattern, so no new story is needed. However, its description still claimed the user must apply inline styling for >10 levels, which is stale after this fix. Updated in a870a54 to note the library now auto-handles >10 levels and the story stands as an override example.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: When tree exceeds 10 levels , an error is displayed indentation

3 participants