Issue 36702 asset picker - #36944
Conversation
- Extracted DotFolderListView component and related models/constants/mocks into the new `@dotcms/ui` library. - Updated imports in the Content Drive shell to reference the new location. - Introduced `selectionMode` input to support both single and multiple selection. - Re-exported necessary symbols from `@dotcms/ui` in the Content Drive UI for compatibility. - Added tests for the new component and selection mode functionality. This migration aims to share the folder/asset list component between Content Drive and future AssetPicker implementations, enhancing code reuse and maintainability.
github-actions
Bot
added
the
Area : Frontend
|
Claude finished @nicobytes's task in 2m 41s —— View job Review — rechecked at
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the existing Folder List View used by Content Drive into a reusable presentational component in @dotcms/ui, adding a single-selection mode intended for the upcoming AssetPicker (while preserving current Content Drive behavior via the default multiple selection mode).
Changes:
- Moved Folder List View domain-agnostic types/constants into
@dotcms/uiand re-exported them from the Content Drive UI package for compatibility. - Added
selectionMode: 'single' | 'multiple'support to the table (checkboxes in multiple mode, radios in single mode) and normalized emitted selections to an array. - Updated Content Drive shell imports and updated/extended unit tests accordingly.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| core-web/libs/ui/src/lib/components/dot-folder-list-view/models.ts | Introduces shared column typing and selectionMode model for the Folder List View. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/constants.ts | Defines header column config and drag MIME type local to the component folder. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/mocks.ts | Moves test mocks alongside the component. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/dot-folder-list-view.component.ts | Adds selectionMode input, normalizes selection output, and updates internal imports to local UI sources. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/dot-folder-list-view.component.html | Switches checkbox vs radio rendering based on selectionMode and updates selection binding. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/dot-folder-list-view.component.scss | Fixes relative SCSS imports to match the libs layout. |
| core-web/libs/ui/src/lib/components/dot-folder-list-view/dot-folder-list-view.component.spec.ts | Updates tests for the new selection model and adds coverage for single-selection behavior. |
| core-web/libs/ui/src/index.ts | Exposes Folder List View component + related models/constants from @dotcms/ui. |
| core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/models.ts | Removes Folder List View column typing now owned by @dotcms/ui. |
| core-web/libs/portlets/dot-content-drive/ui/src/lib/shared/constants.ts | Removes list-view constants now owned by @dotcms/ui. |
| core-web/libs/portlets/dot-content-drive/ui/src/index.ts | Re-exports the Folder List View API surface from @dotcms/ui for Content Drive consumers. |
| core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.ts | Updates imports to use @dotcms/ui for the presentational list component/types. |
| core-web/libs/portlets/dot-content-drive/portlet/src/lib/dot-content-drive-shell/dot-content-drive-shell.component.spec.ts | Aligns test imports with the updated component export location. |
- Introduced new utility functions for managing folder hierarchies, including `getFolderHierarchyByPath` and `getFolderNodesByPath`, to improve folder navigation and loading in the content drive. - Added `folder-tree-load.utils.ts` and `folder-tree.utils.ts` files to encapsulate the new logic. - Implemented comprehensive unit tests for the new utilities to ensure functionality and reliability. - Updated existing services to utilize the new utilities, enhancing code organization and maintainability. These changes aim to streamline folder management and improve the user experience in the content drive interface.
Extracts the content-type, language, and search filter components (plus the chip-filter/list-item primitives and upload button) out of the content-drive portlet into @dotcms/ui so they can be shared with the AssetPicker. Store- specific logic stays behind thin adapter components in the portlet.
- serve target lacked a dependsOn, so dotcms-webcomponents could be stale or missing when dotcms-ui starts serving - webcomponents build target was missing outputs, preventing Nx from caching/detecting its build artifacts correctly
- Introduces DotAssetPickerStore in @dotcms/ui to power the upcoming AssetPicker dialog with a search request builder mirroring Content Drive's, but with no router/URL coupling so it can run inside a dialog over Edit Contentlet without corrupting host navigation. - Relocates ALL_FOLDER/SYSTEM_HOST_ID out of the Content Drive UI library into shared dot-folder-tree constants so both Content Drive and the new picker consume a single source.
…o issue-36702-asset-picker
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 78 changed files in this pull request and generated no new comments.
Suppressed comments (1)
core-web/libs/ui/src/lib/components/dot-asset-picker/store/features/with-asset-folder-tree.feature.ts:97
loadFolderssetsfoldersStatustoERRORincatchError, but then thesubscribeblock unconditionally patches it back toLOADED(becausecatchErrorreturns an empty array). This masks folder-tree failures and makes the UI indistinguishable from a successful empty tree.
The AssetPicker (browse/pick a single asset) now composes the dropzone, upload-type selector, and folder sidebar/toolbar that Content Drive already had, so both features share one implementation instead of duplicating upload flow logic. - Move `dot-content-drive-dropzone` and the upload-type-selector dialog out of the Content Drive portlet into `@dotcms/ui` as `DotUploadDropzoneComponent` and `DotUploadTypeSelectorComponent`, decoupled from `DotContentDriveStore` (folder/drag-state now passed via inputs/outputs) - Add `DotAssetPickerComponent` with sidebar/toolbar subcomponents, wiring the shared dropzone, upload selector, and folder tree to a new `DotAssetPickerStore` - Update Content Drive shell to consume the relocated shared components and derive drag/target-folder state locally
Adds a global last-used-path store, a config builder that translates Edit Content field type (File/Image) into picker filters, and server-side base-type narrowing for the content type filter so restricted hosts don't page through mostly-discarded results.
- Add #assetPickerPending guard so a slow site lookup can't be raced by a second click before #dialogRef is set, and release it on every close (success, cancel, or lookup error) so the picker can reopen afterward. - Rethrow DotDataException as-is in SiteSearchAPIImpl.defaultIndexName instead of re-wrapping it, preserving the original exception type.
…to issue-36702-asset-picker
Reproduces the NullInjectorError the picker still throws in the binary-field custom-element host, which bootstraps with no Router and none of the app-shell providers. b27ab75 added DotContentTypeService and DotHttpErrorManagerService to the picker's own providers. That closes the first half only: DotContentTypeService just needs HttpClient, but DotHttpErrorManagerService transitively pulls in DotAlertConfirmService, DotRouterService (-> Router) and DotEventsSocket. The chain cannot be closed with component-level providers, since provideRouter returns EnvironmentProviders and cannot go in a component's providers array. Skipped on purpose: the failure is the open finding, not a flaky test. Un-skip it when the store stops depending on DotHttpErrorManagerService and reports errors through the MessageService toast the picker already owns for upload and confirm failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review feedback status — rechecked at
|
| Finding | Verified at this HEAD |
|---|---|
🟠 publish() nulls the tree highlight after a sidebar search, retargeting uploads to the site root |
with-asset-folder-tree.feature.ts:222 unchanged — still findNodeByKey(folders, selectedKey) ?? null |
🟠 tree-folder.utils.spec.ts gutted on unchanged production code |
still 37 lines vs 681 on main, and still does not import tree-folder.utils |
🟡 "Select Existing File" gives no feedback when getCurrentSite() fails |
still a silent return; only the new guard flag was added there |
🟡 $selection() !== undefined vs ?? |
dot-folder-list-view.component.ts:313 unchanged |
🟡 i18n 'change' resolves to lowercase change |
dot-workflow-push-publish.component.html:95 unchanged |
One thing worth a separate look
Two of the findings above (SiteSearchAPIImpl, and the 'Change' → 'change' i18n key) are both merge-resolution damage from the same commit (5624a082d4), not intentional changes — main has 'Change' on that line. Since two independent regressions came in through one automated conflict resolution, a manual pass over the backend and CI files in git diff origin/main before merge seems worthwhile; I was not able to complete that sweep myself.
Carrying these across from the Content Drive copy. Each one records a bug that was already fixed once, so losing them makes it cheap to reintroduce: - $columns anchors extras to where "type" sits in the CANONICAL order, not its rendered index. Keying off the rendered index appended the extras when Type was hidden, putting every extra cell one heading early. - the item-title `title` attribute is what keeps truncated text reachable on hover. - the contentType cell truncates so a long variable name cannot grow the row past its neighbours. - the locked-by-others icon is coloured rather than a different glyph on purpose. The title rationale is reworded rather than copied verbatim: that column is no longer a fixed width on this branch, it absorbs the leftover, so the clipping now comes from the cell's `minmax(0,1fr)` track. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @nicobytes's task in 1m 15s —— View job I'll analyze this and get back to you. |
Wiring the Story Block to DotAssetPickerComponent means EditorModalService now holds a static reference to it, so the picker and its subtree (folder list table, folder tree, splitter, content-type filter, Content Drive store) land in the initial chunk. It replaced DotBrowserSelectorComponent, which was considerably lighter, pushing this app to 4.08 MB against a 4.00 MB hard limit. Warning 3.5mb -> 4.25mb, error 4mb -> 4.5mb. Note this is a budget raise, not an optimization: every consumer of @dotcms/new-block-editor now ships the picker in its initial bundle. The alternative is loading the picker through a dynamic import() in openAssetPicker so its subtree moves to a lazy chunk, which would keep the original budget. Left as a follow-up rather than folded in here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The picker threw NullInjectorError and rendered blank when opened from a File/Image field in the legacy content editor, a regression against DotBrowserSelectorComponent, which used root-provided services only. Providing the missing services on the component (b27ab75) only moved the failure one link along. DotHttpErrorManagerService transitively needs DotHttpErrorManagerService +-- DotAlertConfirmService -> ConfirmationService +-- DotMessageDisplayService -> DotRouterService -> Router, DotEventsSocket +-- DotRouterService -> Router and dotcms-binary-field-builder bootstraps with no Router at all -- the same reason GlobalStore was kept out of DotFileFieldComponent. It cannot be closed by plumbing either: provideRouter returns EnvironmentProviders and cannot go in a component's providers. So drop the dependency instead. The store now records failures as `requestError` state (6 call sites across with-asset-browse and with-asset-folder-tree) and the picker component toasts them through the MessageService it already owns for upload and confirm failures. That also puts it back in line with the repo rule that a store holds data, not UI. DotContentTypeService stays in providers -- it only needs HttpClient. The legacy-host spec added in ec5aacc is un-skipped and passing. Its provider set now mirrors app.module.ts exactly rather than a subset, so it fails on anything router-bound being reintroduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two changes pushed that reviewers should know about (
|
Both sides independently moved dot-chip-filter and dot-filter-list-item from dot-content-drive/ui into @dotcms/ui, so git saw add/add on five files with no common ancestor and conflicted them whole. Resolved by reconciling, not by picking a side -- the previous automated merge on this branch (5624a08) picked sides and silently reverted two fixes. What main brought that had to survive: - emptyLabel on dot-chip-filter. Its only consumer is main's new Experiments portlet, so resolving these with "ours" would have dropped the input and broken that portlet's build. - LISTBOX_OPTION_HEIGHT in the theme. Main solved the same problem this branch solved by measuring: it fixes every listbox option to Lara's 0.625rem/1rem and exports the resulting 40.6 so virtual scrollers stop re-measuring it. That is why main dropped py-3 from dot-filter-list-item. So instead of the planned "move the padding into CHIP_FILTER_LISTBOX_PT", this adopts main's approach: - dot-filter-list-item keeps main's version, without py-3. - CHIP_FILTER_LISTBOX_PT drops its --p-listbox-option-padding: 0 1rem override, which would otherwise fight the theme and make every virtual scroller's row height disagree with what is rendered. Its remaining declarations now duplicate the theme; left in place rather than widening a merge commit into a refactor. - dot-content-type-filter takes LISTBOX_OPTION_HEIGHT instead of its own hardcoded 40.6, carrying main's improvement to where the logic now lives. The portlet's content-type and language filters stay as this branch's thin store adapters over the shared @dotcms/ui components -- verified main added no behaviour to them beyond dropping the same PT usage. Also reverts 'change' -> 'Change' in dot-workflow-push-publish: leftover damage from 5624a08, the same merge that dropped the SiteSearchAPIImpl passthrough. change=change is lowercase, so the timezone toggle rendered "change" next to "Hide". Verified: ui 1343, content-drive 1147, edit-content 2243, new-block-editor 87, users 30, publishing-queue 255 -- all passing. Lint clean across the six affected projects. dotcms-block-editor production build passes. git diff origin/main outside core-web is Language.properties only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The upload success toast showed literal `<b>` tags: the picker used a bare
`<p-toast />`, whose default PrimeNG template renders `summary`/`detail` as
escaped text, while `content-drive.add-dotasset-success-detail` bolds the file
name and base type.
Content Drive already solved this with an inline `#message` template, so rather
than copy it — twelve lines of PrimeNG-internal class names and
`data-pc-section` attributes, duplicated — extract it as `dot-toast` in
`libs/ui` and use it in both places. Net -14 lines.
The extracted version modernizes what it moves: `pTemplate="message"` becomes
PrimeNG 21's `#message`, `innerHtml="{{ }}"` becomes `[innerHtml]`, and the
summary/detail wrappers become `div`s to match PrimeNG's own template. The
`MessageService` stays with the consumer, so each outlet keeps its own message
stream — the picker's dialog and the portlet shell do not share one.
`ToastModule` and `DotSeverityIconComponent` drop out of both hosts, and
`PICKER_REAL_IMPORTS` in the picker spec is updated so the list keeps matching
what the component actually composes.
Specs cover the regression directly (markup renders as elements, not text) and
pin the sanitization that makes `[innerHtml]` safe for backend error text.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This pull request introduces a set of improvements and refactorings to the folder tree utilities and related data-access APIs, aimed at enhancing code reuse, maintainability, and consistency across the Content Drive and Host Folder Field features. The changes include moving folder tree logic into shared utilities, updating service providers, and aligning data models.
Core refactoring and utility extraction:
folder-tree.utils.tsandfolder-tree-load.utils.tsin@dotcms/data-access, centralizing the logic for building, loading, and paginating folder trees. This includes new functions such asgenerateAllParentPaths,createTreeNode, andbuildTreeFolderNodes, as well as utilities for paginated loading and "Load more" node handling. [1] [2]API and provider updates:
DotContentDriveServiceto use Angular'sprovidedIn: 'root'for global availability, and removed it from route-level providers. This ensures the service can be used from dialogs and other contexts without explicit injection in every route. [1] [2]Imports and dependency cleanup:
@dotcms/data-accessinstead of local utility definitions, and cleaned up duplicate or outdated imports for components such asDotFolderListViewComponent. [1] [2] [3] [4]Data model alignment:
TreeNodeContentDatamodel to include additional metadata fields (inode,defaultBaseType,fromTable) for improved folder selection and upload behavior in the Content Drive and AssetPicker.Constants and type re-exports:
@dotcms/data-access. [1] [2] [3] [4]These changes improve code maintainability, reduce duplication, and ensure that folder tree behaviors remain consistent across all consuming features.
This PR fixes: #36702
AssetPicker series — all seven sub-issues are resolved in this PR:
Fixes #36831 — AssetPicker 1/7: Migrate DotFolderListView to @dotcms/ui
Fixes #36832 — AssetPicker 2/7: Lift folder-tree helpers and harden drive data-access
Fixes #36833 — AssetPicker 3/7: Decouple Content Drive toolbar chrome for AssetPicker reuse
Fixes #36834 — AssetPicker 4/7: Headless browse store (no URL) for AssetPicker
Fixes #36835 — AssetPicker 5/7: Build DotAssetPicker shell in @dotcms/ui
Fixes #36836 — AssetPicker 6/7: File/Image entry configs, silent mime, CT limits, global last path
Fixes #36837 — AssetPicker 7/7: Wire AssetPicker into Edit Content File/Image fields
This PR fixes: #36702