Skip to content

Tree: Increase child page size to 100 - #23585

Merged
nielslyngsoe merged 12 commits into
v17/devfrom
v17/bugfix/tree-paging-size
Aug 12, 2026
Merged

nielslyngsoe merged 12 commits into
v17/devfrom
v17/bugfix/tree-paging-size

Conversation

@madsrasmussen

@madsrasmussen madsrasmussen commented Aug 7, 2026 •

Copy link
Copy Markdown
Member

Resume
This PR increases the tree's child page size from 50 to 100. It also adds a large fixture to the Kitchen Sink mock set. Loading it revealed several performance issues in tree rendering. This PR addresses some low-hanging fruit, but unfortunately, the main problem of having too many DOM nodes will need to be addressed later.

The PR consists of three things:

1. Page size 50 → 100
2. Mock fixture to test at scale

New Empty Page document type (no properties, allowed at root, allows itself as a child) plus a generated page tree in page-tree.data.ts: Page 1 in the content root with 1,000 children, and along the first branch the first ten children of each page get 500 children of their own, down to level 5 — 16,001 documents.

3. Per-item work removed

  1. Identity guards in setItem and setTreeItem. Skip re-running setup when handed the object already held.
  2. One navigationend listener per tree instead of one per item.
  3. Removed the per-item entity-action subscription
  4. Stable repeat() keys

What this does not fix

When loading 500–1000 tree items, you will start to notice staggered rendering, and it gets worse as more items are loaded.

Here are a few numbers from a Claude performance test in the mocked back office:

Items loaded Time to render the next 100 Median gap per item
100 411 ms 5.4 ms
500 911 ms 10.2 ms
900 1,182 ms 12.6 ms

Our JavaScript accounts for ~0% of that span (0.0–0.4 ms per 100 items, zero long tasks). The cost is per-item DOM and browser work that scales with list size — at 900 items the tree is 20,910 elements, about 23 per row. Expanding several branches at once also serializes: five branches with all data back in ~102 ms, but the fifth branch's first item appeared 14 seconds after the click.

How to test

  1. npm run dev:mock
  2. Switch the mock data set to Kitchen Sink (header dropdown), go to Content
  3. Expand Page 1 — 100 children load per page (was 50); use Load more to page through 1,000
  4. Expand down through Page 1.1 → 1.1.1 → 1.1.1.1 — five levels, 100 per page at each
  5. Open a document and check the correct tree item is highlighted; navigate between several
  6. Rename a document and save — the tree label updates in place
  7. Check entity actions still appear in the classic tree, the card view and the table view (Settings → Document Types → Children has all three)

Notice: The "sort children" functionality hasn’t been implemented in the mocked back office, so it needs to be tested in a real project.

Add a property-less "Empty Page" document type and a generated page tree dataset to the kitchen-sink mocks. This wires a deep, wide document hierarchy into the mock document data so tree pagination and deep expansion scenarios can be exercised more realistically.
Replaced the dedicated tree-item entity action manager with a direct, lazy observable in `UmbTreeItemApiContextBase` and marked `hasActions` as deprecated (scheduled for removal in v19). The default tree item card now always renders `<umb-entity-actions-bundle>` so action visibility is decided by resolved permissions/conditions instead of a pre-check that only verified registered manifests.
@madsrasmussen
madsrasmussen marked this pull request as ready for review August 7, 2026 08:42
Copilot AI lite review requested due to automatic review settings August 7, 2026 08:42
@claude

claude Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Claude finished @madsrasmussen's task in 10m 12s —— View job


PR Review

Target: origin/v17/dev · Based on commit: e43777e5 · Skipped: 3 noise files (document-type.data.ts, document.data.ts, page-tree.data.ts)

Increases tree child page size from 50 to 100, adds a 16,001-document Kitchen Sink fixture, and removes several categories of per-item work: a dedicated navigationend listener, a per-item entity-action subscription, and redundant setup on same-object re-renders — plus stable repeat() keys to prevent needless DOM churn.

  • Modified public API: UmbTreeItemActiveManager — new activeTrail, setActiveTrail, getActiveTrail, removeActiveTrailIfMatch, isCurrentLocation members added; active, setActive, getActive, removeActiveIfMatch deprecated. UmbTreeItemApi interface — hasActions marked deprecated.
  • Affected implementations (outside this PR): No external code in src/ uses the deprecated activeManager.*Active* methods. default-tree-item-card.element.ts stopped observing hasActions (now always renders <umb-entity-actions-bundle> directly, which self-hides when there are no actions).
  • Breaking changes: UmbTreeItemEntityActionManager deleted — not exported from any index.ts, so not a public breaking change. All deprecated members retain their previous behaviour and follow the two-mechanism pattern (JSDoc @deprecated + UmbDeprecation runtime warning, removal scheduled for v19). ✓

Suggestions

  • tree-item-api-context-base.ts:291: The local variable path inside #applyActiveState is an Array<UmbEntityModel> (entity trail), not a URL path string — it shares a name with the class's readonly path: Observable<string> (the URL path). Consider renaming it activeTrail or trail to avoid the mental collision.

Approved with Suggestions for improvement

Good to go. The reactive chain — navigationend debounced once per tree → #currentLocation state → isCurrentLocation combineLatest → #isCurrentLocation field → #applyActiveState → setActiveTrail — is clean, and the deprecation shims correctly delegate and warn. The test coverage for the listener lifecycle and the keyed-rendering behaviour is a welcome addition.


Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the backoffice tree UI to load more children per page (50 → 100) and reduces per-item runtime work during tree rendering, supported by a new large Kitchen Sink mock fixture to exercise pagination and deep expansion scenarios.

Changes:

  • Increased default tree child pagination size to 100 across relevant tree contexts/managers.
  • Reduced per-item overhead in tree rendering (stable repeat() keys, identity guards, move navigationend handling to a tree-scoped active manager, remove per-item entity-action subscription).
  • Added a large Kitchen Sink mock dataset (16,001 documents) and a minimal “Empty Page” document type to test tree behavior at scale.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Umbraco.Web.UI.Client/src/packages/core/tree/view/classic/classic-tree-view.element.ts Uses stable Lit repeat() keys for root item rendering.
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-entity-action.manager.ts Removed per-item entity-action manager (eliminates per-item registry observation).
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-children.manager.ts Increases default take size to 100.
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-base/tree-item-element-base.ts Adds identity guard for item setter and uses stable repeat() keys for child items.
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-base/tree-item-element-base.test.ts Adds tests covering item identity guard and stable keyed rendering behavior.
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item/tree-item-base/tree-item-context-base.ts Updates tree item context to set take size to 100.
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item-card/default/default-tree-item-card.element.ts Removes reliance on deprecated hasActions; always renders <umb-entity-actions-bundle> (which self-hides when empty).
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item-api/tree-item-api.interface.ts Marks hasActions as deprecated (v17 → remove in v19).
src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item-api/tree-item-api-context-base.ts Reworks active-state tracking to use the tree active manager’s location observable; deprecates hasActions access with guidance.
src/Umbraco.Web.UI.Client/src/packages/core/tree/default/default-tree.context.ts Updates default tree context to set take size to 100.
src/Umbraco.Web.UI.Client/src/packages/core/tree/active-manager/tree-active-manager.ts Introduces tree-scoped navigationend listener + isCurrentLocation(); renames “active” concept to activeTrail with deprecated shims.
src/Umbraco.Web.UI.Client/src/packages/core/tree/active-manager/tree-active-manager.test.ts Adds coverage for isCurrentLocation() and listener lifecycle; updates tests for renamed API.
src/Umbraco.Web.UI.Client/mocks/data/sets/kitchen-sink/page-tree.data.ts Adds large generated document tree fixture for scale testing.
src/Umbraco.Web.UI.Client/mocks/data/sets/kitchen-sink/document.data.ts Includes the new page-tree fixture in the Kitchen Sink document dataset.
src/Umbraco.Web.UI.Client/mocks/data/sets/kitchen-sink/document-type.data.ts Adds “Empty Page” document type used by the large fixture.

@claude claude Bot added area/frontend category/performance Fixes for performance (generally cpu or memory) fixes category/ux User experience labels Aug 7, 2026
Updated Lit `repeat` key generation in both classic tree view and tree item base to use `${entityType}:${unique}` instead of direct string concatenation. This prevents accidental key collisions when different values could produce the same concatenated key and improves stable rendering behavior.
# Conflicts:
#	src/Umbraco.Web.UI.Client/src/packages/core/tree/tree-item-card/default/default-tree-item-card.element.ts
@nielslyngsoe
nielslyngsoe enabled auto-merge (squash) August 12, 2026 13:42
@sonarqubecloud

Copy link
Copy Markdown

@nielslyngsoe
nielslyngsoe merged commit 253bd6c into v17/dev Aug 12, 2026
32 of 33 checks passed
@nielslyngsoe
nielslyngsoe deleted the v17/bugfix/tree-paging-size branch August 12, 2026 14:35
This was referenced Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/frontend category/performance Fixes for performance (generally cpu or memory) fixes category/ux User experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants