Skip to content

34420 unify component#36172

Open
nicobytes wants to merge 37 commits into
mainfrom
34420-unify-component
Open

34420 unify component#36172
nicobytes wants to merge 37 commits into
mainfrom
34420-unify-component

Conversation

@nicobytes

@nicobytes nicobytes commented Jun 15, 2026

Copy link
Copy Markdown
Member

This pull request introduces significant updates to the handling and testing of binary fields (file/image uploads) in the content editing experience. The main focus is on unifying the binary field implementation under the new dot-edit-content-file-field component, updating related imports and tests, and improving Playwright E2E test coverage for both the new and legacy editors. The changes also include new and refactored E2E test helpers for binary fields, and adjustments to test data and selectors.

Component and Module Refactoring:

  • The binary field implementation is unified under the dot-edit-content-file-field component, replacing the previous dot-edit-content-binary-field and its wrapper throughout the app and tests. This includes updating Angular module imports, dependency injection, and testbed configuration to use the new component and its bridge (DotBinaryFieldCeBridgeComponent). [1] [2] [3] [4] [5] [6] [7] [8]

E2E Test Enhancements:

  • Adds comprehensive Playwright E2E tests for the binary field image editor in both the new and legacy editors, including creation of content types, file import, and image editing workflows.
  • Introduces new E2E page objects for the legacy content editor and binary field, improving test structure and maintainability. [1] [2] [3]

Test Helper Improvements:

  • Refactors and extends the binary field test helper to support new selectors, actions (like image upload and editor opening), and improved assertions for button visibility and file handling. [1] [2] [3] [4] [5]

Test Data and Selector Updates:

  • Updates the default import-from-URL image to a stable 800x800 PNG for more robust image editor testing.
  • Aligns data-testid and DOM selectors in helpers with the new unified binary field implementation.

These changes collectively modernize the binary field experience, improve test coverage and reliability, and streamline the codebase for future enhancements.

This PR fixes: #34420

nicobytes added 11 commits June 15, 2026 10:54
…rapper

- Added a new `DotLegacyImageEditorLauncherService` to manage the image editor dialog lifecycle.
- Introduced `DotLegacyImageEditorDialogComponent` for rendering the image editor iframe.
- Updated `DotBinaryFieldWrapperComponent` to utilize the image editor service and handle value updates.
- Enhanced the binary field HTML template to include an image editor toggle.
- Added unit tests for the new image editor dialog and launcher service.

This implementation allows users to edit images directly within the binary field, improving the content editing experience.
…message event handling

- Added `dispatchSpy.restore()` calls to ensure proper cleanup after each test case.
- Updated the origin check in the message event test to use a variable for invalid origins, improving test reliability.
- Enhanced assertions to ensure that only valid events trigger the expected behavior.

These changes improve the robustness of the unit tests for the `DotLegacyImageEditorLauncherService`.
- Introduced a new specification for the binary field image editor, verifying the visibility of the Edit button and the opening of the legacy Dojo Image Editor in both new and legacy content editors.
- Created helper classes for managing interactions with the binary field and legacy binary field within the tests.
- Updated the binary field helper to include methods for image upload and editor interaction.
- Enhanced the legacy binary field helper to support image editing in the legacy editor.

These changes improve the testing coverage for the binary field image editor functionality, ensuring a seamless user experience across different content editor versions.
…ar' of github.com:dotCMS/core into 34420-epic-image-editor-feature-parity-migrate-to-angular
…ame security handling

- Changed the default image URL for import-from-URL tests to a stable 800x800 PNG from `placehold.co`.
- Updated the `DotLegacyImageEditorDialogComponent` to use the SafeUrlPipe for iframe URL sanitization, improving security.
- Adjusted related documentation to reflect the new image URL and its dimensions.

These changes enhance the reliability and security of the binary field image editor functionality.
…file

- Deleted the `binary-field-image-editor.md` specification file as it is no longer needed.
- This cleanup helps streamline the project documentation and maintain focus on current testing strategies.

No functional changes were made to the codebase.
- Replaced the `DotEditContentBinaryFieldComponent` with `DotEditContentFileFieldComponent` to unify the handling of binary and file fields.
- Updated related tests and helper classes to reflect the new component structure.
- Removed outdated binary field HTML, SCSS, and spec files to streamline the codebase.

These changes enhance the maintainability and consistency of the binary field functionality within the content editing framework.
- Introduced `LegacyDialogImageEditorLauncher` to replace the previous Dojo-based image editor integration, allowing the legacy image editor to be opened in a PrimeNG dialog.
- Updated the `DotFileFieldComponent` to conditionally expose the image editor action only for binary fields.
- Modified related tests and components to accommodate the new image editor launcher and ensure proper functionality.
- Enhanced the styling of the file field component for better layout consistency.

These changes improve the user experience by modernizing the image editing workflow within the content editing framework.
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Jun 15, 2026
@claude

claude Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @adrianjm-dotCMS's task in 1m 13s —— View job


Rollback Safety Analysis

  • Read rollback-unsafe categories reference
  • Get full PR diff
  • Analyze diff against all unsafe categories
  • Apply appropriate label

Findings

All 100 changed files fall into three categories:

Area Files Rollback risk
Angular/TypeScript frontend (core-web/) 97 None — UI-only component refactoring, no backend contract changes
Language.properties 1 None — purely additive (3 new i18n keys added; N-1 simply omits them)
justfile 1 None — local dev tooling

Checked against every category:

Category Verdict
C-1 Structural data model change ✅ No runonce tasks, no DB schema changes
C-2 Elasticsearch mapping change ✅ No changes to ESMappingAPIImpl, ESMappingUtilHelper, or ESMappingConstants
C-3 contentlet_as_json model version bump ✅ No changes to ImmutableContentlet or CURRENT_MODEL_VERSION
C-4 DROP TABLE / DROP COLUMN ✅ No DDL drops
H-1 One-way data migration / backfill ✅ No data transformation tasks
H-2 RENAME TABLE / RENAME COLUMN ✅ No SQL renames
H-3 Primary key restructuring ✅ No PK changes
H-4 New content type field type ✅ Binary/File/Image field types already exist on the backend; this PR only touches the Angular rendering layer
H-5 Binary storage provider change ✅ No changes to FileStorageAPIImpl, StoragePersistenceProvider, or getBinaryMetadataVersion()
H-6 DROP PROCEDURE / DROP FUNCTION ✅ No stored procedure changes
H-7 NOT NULL column without default ✅ No column additions
H-8 VTL viewtool contract change ✅ No changes to ViewTool classes, toolbox.xml, or returned DTO types
M-1 Non-broadening column type change ✅ No column type changes
M-2 Push publishing bundle format change ✅ No Bundler/Handler class changes
M-3 REST / GraphQL API contract change ✅ No REST endpoint Java changes; Angular admin UI updates only
M-4 OSGi plugin API breakage ✅ No OSGi-exported interface changes

Result: Safe to Rollback — this PR is a pure frontend refactor with no database, Elasticsearch, serialization, or API contract changes.

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex Review — openai.gpt-5.5

**[> [🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/binary-field/binary-field.spec.ts:184 — The test now always expects the dotAI button to be disabled, but the removed isAiButtonEnabled()/test.skip(...) guard handled environments where the dotAI plugin is installed. This makes the suite fail in plugin-enabled environments even though the behavior is valid there.


Run: #27565980035 · tokens: in: 19850 · out: 3914 (reasoning: 3814) · total: 23764

- Updated the binary field image editor tests to reflect the new legacy image editor dialog behavior.
- Added tests to verify that the "Edit image" button is hidden for non-image file uploads and that the confirmation popup appears when removing a file.
- Improved the existing test for editing binary content to ensure it displays the preview without server errors.

These changes enhance the test coverage and reliability of the binary field image handling functionality within the content editing framework.
…nd image fields

- Refactored the required field tests for binary, file, and image fields to use a consistent structure with `beforeEach` and `afterEach` hooks for setup and teardown.
- Improved test readability and maintainability by encapsulating the creation and deletion of content types within dedicated hooks.
- Ensured that error handling for required fields is properly tested across all relevant field types.

These changes enhance the clarity and organization of the end-to-end tests for required fields in the content editing framework.
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

🤖 Bedrock Review — deepseek.v3.2

[🔴 Critical] core-web/libs/data-access/src/lib/dot-messages/dot-messages.service.ts:70init() method now returns an Observable<void> but the load$ observable is subscribed to with take(1) before returning, causing a potential race condition. Callers expecting the observable to emit after initialization may receive a completed stream that never emits. This breaks the new async initialization pattern.

[🔴 Critical] core-web/libs/data-access/src/lib/dot-messages/dot-messages.service.spec.ts:60 — Test 'should call languages endpoint with default language and set them in local storage' now uses firstValueFrom but doesn't wait for the promise before assertions. The test may pass before the HTTP call completes, leading to false positives.

[🟠 High] core-web/libs/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.ts:60 — Removed import of DotBinaryFieldWrapperComponent but the component is still referenced in the FIELD_TYPES_COMPONENTS object (line 183 in the spec file). This will cause a runtime error when rendering a BINARY field type.

[🟠 High] core-web/libs/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.html:103-111 — Removed the dot-binary-field-wrapper case block without adding a corresponding dot-edit-content-file-field case for fieldTypes.BINARY. The BINARY field type will now fall through to the default case, rendering nothing.

[🟠 High] core-web/libs/edit-content/src/lib/components/dot-edit-content-field/dot-edit-content-field.component.spec.ts:183 — The test configuration for FIELD_TYPES.BINARY still references the old DotBinaryFieldWrapperComponent. It should be updated to use DotEditContentFileFieldComponent with the correct providers.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts:74expectAiButtonDisabledWithTooltipWhenApplicable() method returns early if the AI button is enabled, making the test a no-op. This could hide a regression where the button should be disabled but isn't. The test should at least verify the button's state.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field.spec.ts:176 — Test 'disabled Generate With dotAI button shows tooltip when AI plugin not installed' calls field.expectAiButtonDisabledWithTooltipWhenApplicable() which may pass silently if the button is enabled (i.e., dotAI is installed). The test should be skipped or marked as conditional, not silently pass.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/utils/contentListingNavigation.ts:34isOnAngularContentRoute function uses regex /#\/content\// which may match other routes (e.g., /#/content-types). This could cause unnecessary navigation reloads. Should be more specific: /#\/content\/[^/]+$.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/pages/legacyEditContentForm.page.ts:23getLegacyContentFrame() uses matchingFrames.at(-1) to get the last matching frame. This assumes the most recent frame is the correct one, which could be flaky if multiple edit frames exist. Should have a more robust selection logic (e.g., wait for a specific element inside the frame).

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/relationship-field/helpers/relationship-field.ts:151 — New method getHeaderTexts() returns lowercased headers, but callers compare with partial strings (e.g., includes('title')). This is case-insensitive but may cause confusion; should document that comparisons must be lowercased or handle case-insensitivity explicitly.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/relationship-field/helpers/relationship-field.ts:173dragRowToPosition() uses hardcoded steps: 10 in mouse movement. This may not be sufficient for drag‑and‑drop to register in all environments; should be configurable or increased.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/relationship-field/helpers/select-existing-content-dialog.ts:206expectRowConstrained() and expectRowConstrainedByText() duplicate logic. Consider consolidating to avoid maintenance drift.

[🟡 Medium] core-web/apps/dotcms-ui-e2e/src/tests/login/translations.spec.ts:28 — Removed assert import but the test still uses assert in comment. No functional issue, but leftover dead code comment.


Run: #27984169620 · tokens: in: 21276 · out: 1120 · total: 22396

@nicobytes

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

…ownload functionality

- Updated the dot-file-field-preview component to include a download button for temporary preview files.
- Improved the logic for generating download links based on the presence of reference URLs.
- Adjusted styles for better responsiveness and usability across different screen sizes.
- Added unit tests to verify the new download functionality and ensure proper behavior when reference URLs are missing.
- Regenerated `spec.json` to include new API paths and updated parameters for the `/api/v1/categories` endpoint, improving API documentation.
- Enhanced Jest configuration by adding a module name mapper for `@primeuix/motion`, facilitating better testing practices.
- Updated test setup to include additional properties for computed styles, ensuring more accurate mock implementations.
- Introduced a new mock file for `primeuix-motion` to streamline animation testing and improve test reliability.
@nicobytes

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in baa2b30.

@adrianjm-dotCMS

adrianjm-dotCMS commented Jun 17, 2026

Copy link
Copy Markdown
Member

Bug 1: UI Layout Overflow & Missing Edit Option when is a txt file

Editor: Both

Content Type: QA Binary / QA Binary required

Description: There is a visual discrepancy between the demo and the current branch. When creating new content, the content overflows outside the field container boundaries. Additionally, the option to edit the binary field is missing ( I cannot click on it to edit it ) when we are uploading a txt file. Note: This mostly happens during creation
Screenshot 2026-06-17 at 3 16 28 PM

Steps to Reproduce:

  • Go to Content → New → Select Binary field.

  • Add a Txt file and try to edit it by clicking

Expected Result:
The container fits the field elements properly within its boundaries (no text/content overflow). The option to edit the binary field should be accessible if applicable.

**Bug 2: False Positive "Required" Validation on Binary Field **

Editor: Old Editor

Content Type: Binary required

Description:
The binary field triggers a false "This field is mandatory" validation error upon saving, even though a valid file has already been uploaded/added into the field.

Steps to Reproduce:

  • Go to Content → New → Select Binary field.
  • Try to add a file and save it
image

Expected Result:
The contentlet saves successfully since all required fields (Binary field) contain data.

Bug 3: Binary Field Content Not Saving / Dropping Hydration
Related TC: TC-BIN-001 / TC-BIN-006 — Edit existing content (hydration)

Editor: Old editors

Content Type: Binary
Description:
When adding a binary file to a binary field the file data fails to persist. Upon saving and reloading, the field reverts to an empty state. (Only when creating)

Screen.Recording.2026-06-17.at.3.25.16.PM.mov

Steps to Reproduce:

  • Open Content → New → Binary

  • Upload a file to the non-required binaryField.

  • Click Save.

  • Reopen the newly created contentlet.

Expected Result:
The contentlet reopens with the binary file fully hydrated (preview and filename visible, no data loss).

Bug 4: Image Editor Preview Synchronization Delay on Empty Fields

Editor: Both editors

Content Type: Binary

Description:
When interacting with the Image Editor on a brand-new/empty binary field, modifications made inside the PrimeNG dialog editor do not sync back to the main form preview in real-time.

Screen.Recording.2026-06-17.at.3.27.43.PM.mov

Steps to Reproduce:

  • Open Content → New → Binary

  • Upload a small image or import via URL to an empty binary field.

  • Click on Edit image to open the PrimeNG dialog editor.

  • Apply any modification/crop inside the editor and confirm changes.

  • Check the field preview on the main form before saving.

Expected Result:
The modified image steps should immediately reflect on the form's preview container prior to hitting the final save button.

Aditional UI issue: The field icons collide with the command bar.

Screenshot 2026-06-17 at 3 31 22 PM

…nce file editor functionality

- Changed the event emitted on value update from `valueUpdated` to `onInnerValueUpdated` to ensure proper synchronization with the FormControl.
- Added a new method in the file field component to handle value updates directly, improving the reactive form integration.
- Enhanced the file editor component to support additional properties for file uploads, including `uploadType` and `acceptedFiles`.
- Updated the file preview component to emit an event for editing files when the preview is clicked.
- Adjusted styles for better usability and responsiveness in the file preview and editor components.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[EPIC] Image Editor Feature Parity Migrate to Angular

4 participants