Skip to content

36733 unify folder p tree into a shared dotcmsui component - #36848

Open
nicobytes wants to merge 5 commits into
mainfrom
36733-unify-folder-p-tree-into-a-shared-dotcmsui-component
Open

36733 unify folder p tree into a shared dotcmsui component#36848
nicobytes wants to merge 5 commits into
mainfrom
36733-unify-folder-p-tree-into-a-shared-dotcmsui-component

Conversation

@nicobytes

@nicobytes nicobytes commented Jul 31, 2026

Copy link
Copy Markdown
Member

This pull request refactors the folder tree display components in both the edit-content host folder field and the content drive sidebar to use the new DotFolderTreeComponent wrapper instead of the raw PrimeNG p-tree. It unifies the tree node data model to support a new "load-more" sentinel node type, updates related tests, and simplifies the handling of selected nodes and labels.

Component Refactoring and API Unification

  • Replaced all usages of p-tree with the new DotFolderTreeComponent in host-folder-field.component.html and dot-tree-folder.component.html, updating inputs and outputs to match the new API. This includes switching from [value]/[selection] to [folders]/[selectedNode], and updating event handlers and templates accordingly. [1] [2] [3]
  • Updated component and test imports to use DotFolderTreeComponent and new pipes, removing unused PrimeNG modules and pipes. [1] [2] [3] [4]

Tree Node Data Model Enhancements

  • Extended the TreeNodeData type to include a load-more node type, with optional nextPage and remaining properties for pagination support. Updated all relevant code to use this new type and ensure "load-more" nodes are correctly constructed. [1] [2] [3] [4]

Selected Node Handling

  • Changed the selected node input from an array to a single node object in both the sidebar and host folder field, updating tests and internal logic to match. [1] [2] [3] [4]

Load More Node Handling

  • Updated the "load more" node handling to use the new unified node type, changed the event signature for loading more items to accept an optional event, and ensured all "load more" buttons and test IDs are passed through the new component. [1] [2] [3]

Test and Utility Updates

  • Adjusted tests and utility functions to match the new node type, selected node handling, and component structure. This includes updating test expectations for selected nodes and "load more" nodes, and removing obsolete tree styling. [1] [2] [3] [4] [5]

These changes modernize and standardize the folder tree UI components, improve maintainability, and add support for paginated folder loading.

This PR fixes: #36733

- Extracts a reusable dot-folder-tree component so host-folder-field,
  dot-tree-folder, and dot-sidebar consume one shared implementation
  instead of duplicating p-tree config, load-more button, and toggler
  templates.
- Adds a folder-name pipe as the successor to dot-truncate-path, kept
  as a thin deprecated alias for backward compatibility.
- Simplifies selectedNode handling to a single TreeNode instead of an
  array-transform workaround for PrimeNG's selection API.
Introduces a presentational folder-tree component in @dotcms/ui to
consolidate the p-tree chrome (togglers, load-more, node labels)
shared across Content Drive, Browser Selector, and Host Folder Field,
so each consumer can own its data/UX without duplicating tree markup.

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

Refactors the admin UI folder-tree implementations to use a shared, presentational DotFolderTreeComponent in @dotcms/ui, unifying selection handling and introducing a consistent “load-more” sentinel node model across consumers (Content Drive, Browser Selector sidebar, and Host Folder Field).

Changes:

  • Added DotFolderTreeComponent (standalone) that owns the PrimeNG p-tree shell, templates (default/load-more/toggler), selection normalization, and test IDs.
  • Migrated Browser Selector sidebar, Content Drive tree wrapper, and Host Folder Field overlay tree to consume the shared component and the unified node model (including type: 'load-more').
  • Consolidated path-label behavior by promoting FolderNamePipe and deprecating DotTruncatePathPipe as an alias, with corresponding test updates.

Reviewed changes

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

Show a summary per file
File Description
core-web/libs/ui/src/lib/pipes/dot-truncate-path/dot-truncate-path.spec.ts Adds coverage for empty-path behavior in the deprecated alias pipe.
core-web/libs/ui/src/lib/pipes/dot-truncate-path/dot-truncate-path.pipe.ts Deprecates the pipe in favor of FolderNamePipe while keeping compatibility for existing call sites.
core-web/libs/ui/src/lib/pipes/dot-folder-name/dot-folder-name.pipe.ts Clarifies pipe intent/docs for extracting the final path segment.
core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.ts Introduces the shared folder-tree shell component and its unified API (folders/selection/load-more/toggler behavior).
core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.html Implements shared templates for default nodes, load-more button, and custom toggler icons.
core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.scss Centralizes tree toggler mode styling (chevron vs first-only folder icon).
core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.spec.ts Adds Jest/Spectator coverage for selection normalization, outputs, load-more behavior, and default labeling.
core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.ts Switches Browser Selector sidebar to the shared tree and simplifies selection state management.
core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.html Replaces inline p-tree usage with <dot-folder-tree> and wires events/inputs.
core-web/libs/ui/src/index.ts Exports the new shared DotFolderTreeComponent from @dotcms/ui.
core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.ts Refactors Content Drive tree wrapper to delegate the tree shell to DotFolderTreeComponent while keeping drag/drop and “All folders” behavior.
core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.html Replaces Content Drive’s inline p-tree with <dot-folder-tree> and projects the label template for Content Drive–specific markup.
core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.scss Removes tree-specific toggler styling now owned by the shared component.
core-web/libs/portlets/dot-content-drive/ui/src/lib/dot-tree-folder/dot-tree-folder.component.spec.ts Updates tests for the wrapper’s new selected-node shape and wrapper-based rendering.
core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.ts Ensures load-more sentinel nodes also set the PrimeNG TreeNode.type for template matching.
core-web/libs/portlets/dot-content-drive/portlet/src/lib/utils/functions.spec.ts Updates expectations to include the type: 'load-more' field for sentinel nodes.
core-web/libs/portlets/dot-content-drive/portlet/src/lib/components/dot-content-drive-sidebar/dot-content-drive-sidebar.component.spec.ts Aligns sidebar tests with the new single-node selection input.
core-web/libs/edit-content/src/lib/fields/dot-edit-content-host-folder-field/store/host-folder-field.store.ts Updates load-more node creation to include unified data payload for the new shared tree.
core-web/libs/edit-content/src/lib/fields/dot-edit-content-host-folder-field/components/host-folder-field/host-folder-field.component.ts Migrates Host Folder Field overlay tree to the shared component and adjusts scroll-to-selection logic.
core-web/libs/edit-content/src/lib/fields/dot-edit-content-host-folder-field/components/host-folder-field/host-folder-field.component.html Replaces inline p-tree with <dot-folder-tree>, projects label template, and wires load-more via the shared output.
core-web/libs/edit-content/src/lib/fields/dot-edit-content-host-folder-field/components/host-folder-field/host-folder-field.component.spec.ts Updates stubs/expectations to match the wrapper component API and scroll helper access.
core-web/libs/dotcms-models/src/lib/dot-browser-selector.model.ts Extends the shared tree node data model to include load-more with pagination metadata.

</button>
</ng-template>

<!-- LOADING WONT WORK WITH CUSTOM TEMPLATE: https://github.com/primefaces/primeng/blob/v17/src/app/components/tree/tree.ts#L113 -->
Comment on lines 1 to 5
import { Pipe, PipeTransform } from '@angular/core';

import { FolderNamePipe } from '../dot-folder-name/dot-folder-name.pipe';

/**
@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Jul 31, 2026
- Updated the `createFolders` function to return an array of `CreatedFolder` objects instead of void, enhancing the API's usability.
- Modified the `createFolders` type definition in the fixture to reflect the new return type.
- Added export for `ContentDrivePage` in the index file to improve component accessibility.
- Introduced a new `ContentDrive` constant in the portlets utility for better navigation within the application.
@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nicobytes's task in 14m 52s —— View job


Code Review — PR #36848

Reviewed the shared DotFolderTreeComponent and its three consumers (Browser Selector sidebar, Content Drive tree wrapper, Host Folder Field) plus the model/pipe changes.

New Issues

  • 🟡 Medium: libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.ts:113onNodeSelect = output<TreeNodeExpandEvent>(). PrimeNG's (onNodeSelect) emits a TreeNodeSelectEvent, not an expand event. Same for dot-sidebar.component.ts:63 (onNodeSelect) and handleNodeSelect(event: TreeNodeExpandEvent) at :100. Structurally compatible (both carry originalEvent/node), so this is not a runtime bug, but the type is misleading for consumers and diverges from TreeNodeSelectEvent used elsewhere in the tree code. Import from primeng/types/tree for consistency.

  • 🟡 Medium: libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.html:22 and host-folder-field.component.html:281{{ node.label | folderName }}. TreeNode.label is string | undefined in PrimeNG, and FolderNamePipe.transform calls value.split('/'), which throws on undefined. In practice folder/site nodes always carry a label and load-more nodes use a separate template, so this only fires if a node ever lacks a label. This risk existed before (the old dotTruncatePath also called .split), so it is not newly introduced — but since the pipe is now the shared default it would be cheap to guard the pipe input (value?.split(...)) or the template (node.label ?? '').

  • 🟡 Medium: libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.scss:29 — the .folder-all mode is defined in the shared SCSS but nothing ever applies that class: treeStyleClasses() only emits first-only or chevron-only. This is a behavior change for Content Drive's dot-tree-folder, whose default (showFolderIconOnFirstOnly = false) previously rendered folder-all and now renders chevron-only. The only real consumer (dot-content-drive-sidebar) passes [showFolderIconOnFirstOnly]="true"first-only, so there's no visible regression today, and the tests were updated to match. Flagging so the folder-all intent isn't silently lost and the dead CSS block isn't mistaken for live styling. Fix this →

Notes (non-blocking)

  • dot-folder-tree.component.html:46 — comment typo WONTWON'T (also present in the original Content Drive template this was lifted from).
  • dot-truncate-path.pipe.ts:19 — the deprecated alias instantiates new FolderNamePipe() directly. Fine for a pure stateless pipe, but Copilot's suggestion to inline the one-liner (or share a util) is reasonable given the logic is trivial. Non-blocking since it's a temporary migration shim; consider migrating the two remaining dotTruncatePath call sites (site-field.component.*) so the alias can be deleted.

Verify (low confidence)

  • dot-sidebar.component.ts:70-95 — the new #userSelected + effect selection model relies on PrimeNG matching [selection] against the tree nodes. After loadChildren runs structuredClone(store.folders().data) in browser.store.ts:174, the tree nodes are replaced by clones while #userSelected retains the pre-clone reference. Because loadChildren mutates the clicked node in place (node.children = [...]) before the clone, the retained reference and its clone should be deep-equal, so PrimeNG (no dataKey) should keep the highlight — what to verify: manually confirm the selected folder stays highlighted after expanding it and after a folder reload in the file-field browser dialog.

Overall the refactor is clean and the consolidation into @dotcms/ui is sound. No blocking issues found — the items above are type-accuracy, a guard, and a dead-CSS/behavior note.

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

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (8)

core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.ts:113

  • PrimeNG's (onNodeSelect) emits a select event, not an expand event. Typing this output as TreeNodeSelectEvent avoids confusing API consumers and matches other tree consumers in the codebase.
    onNodeSelect = output<TreeNodeExpandEvent>();

core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.ts:63

  • onNodeSelect should be typed as TreeNodeSelectEvent (selection), not TreeNodeExpandEvent (expansion). This helps downstream consumers and matches the JSDoc intent.
    onNodeSelect = output<TreeNodeExpandEvent>();

core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.ts:103

  • handleNodeSelect receives the selection event from dot-folder-tree / PrimeNG (onNodeSelect), so its parameter type should be TreeNodeSelectEvent rather than TreeNodeExpandEvent.
    handleNodeSelect(event: TreeNodeExpandEvent): void {
        this.#userSelected.set(event.node);
        this.onNodeSelect.emit(event);
    }

core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.html:22

  • TreeNode.label is optional in PrimeNG, and FolderNamePipe calls .split() on its input. If any node has an undefined/null label, this will throw at runtime. Guard the pipe input in the template to ensure a string is always passed.
            <span data-testid="tree-node-label" class="font-normal">
                {{ node.label | folderName }}
            </span>

core-web/libs/edit-content/src/lib/fields/dot-edit-content-host-folder-field/components/host-folder-field/host-folder-field.component.html:281

  • This template now uses folderName on node.label, but PrimeNG TreeNode.label can be undefined. Guarding with node.label ?? '' prevents runtime errors from .split() in the pipe if a node ever lacks a label.
                                <span
                                    class="block w-full min-w-0 truncate"
                                    [class.font-semibold]="isSearching"
                                    [class.leading-normal]="isSearching">
                                    {{ node.label | folderName }}
                                </span>

core-web/libs/ui/src/lib/pipes/dot-truncate-path/dot-truncate-path.pipe.ts:22

  • Even as a deprecated alias, dotTruncatePath can still be invoked from templates with null/undefined values. Guard the input so it never throws when delegating to FolderNamePipe.
    private readonly folderNamePipe = new FolderNamePipe();

    transform(value: string): string {
        return this.folderNamePipe.transform(value);
    }

core-web/libs/ui/src/lib/components/dot-folder-tree/dot-folder-tree.component.ts:17

  • DotFolderTreeComponent.onNodeSelect is currently typed as TreeNodeExpandEvent, but PrimeNG emits TreeNodeSelectEvent for selection. Importing the correct event type makes the wrapper API clearer and aligns with existing patterns (e.g. Content Drive sidebar uses TreeNodeSelectEvent).

This issue also appears on line 113 of the same file.

import type { TreeNodeCollapseEvent, TreeNodeExpandEvent } from 'primeng/types/tree';

core-web/libs/ui/src/lib/components/dot-browser-selector/components/dot-sidebar/dot-sidebar.component.ts:13

  • For consistency with other tree consumers (and to get access to TreeNodeSelectEvent), import PrimeNG tree event types from primeng/types/tree instead of primeng/tree.

This issue also appears in the following locations of the same file:

  • line 63
  • line 100
import type { TreeNodeExpandEvent } from 'primeng/tree';

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

Labels

Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Unify folder p-tree into a shared @dotcms/ui component

2 participants