Conversation
- Replaced the usage header with a PrimeNG toolbar for better action handling. - Enhanced loading state with a more structured skeleton layout for site metrics, system configuration, and user activity. - Updated error handling to use PrimeNG card components for a cleaner presentation. - Refactored dashboard content layout to utilize flexbox for better responsiveness. - Adjusted SCSS styles to align with new component structure and improve overall styling consistency. This update enhances the user experience and maintains a modern design approach.
…yling - Adjusted skeleton component heights for better visual consistency. - Added margin utility class to paragraph elements within skeleton templates. - Enhanced SCSS styles for skeleton display and h3 elements to improve layout and readability. These changes aim to refine the user interface and enhance the loading experience in the dot-usage-shell component.
…ttpClient providers
- Introduced DotUsageService to fetch usage summary metrics from the backend API. - Implemented error handling for various HTTP status codes. - Added unit tests for DotUsageService to validate summary retrieval and error handling. - Updated dot-usage-shell component to manage loading and error states using signals. - Refactored component tests to utilize the new service structure. These changes enhance the functionality and reliability of the dot-usage feature, ensuring accurate data retrieval and user-friendly error messages.
…ading state management
…me height management
…SCSS styles for layout consistency
…SCSS styles for layout consistency
…sageToIframe method signature
…itioning calculations for zoom functionality
…nused expanded property from node definitions
…d enhance data structure for container information
…tructure for improved data representation
…dling for improved layout management
…nused service injections for improved code clarity
…handling to scroll to corresponding elements in the editor, improving user navigation experience.
…om levels, enhancing the accuracy of element positioning in the editor.
… for tree nodes, allowing users to reorder rows within the layout. Add event handling for node selection and drop validation to improve user interaction and layout management.
…ve function: implement updateRows method for saving reordered layout rows, enhancing layout management and state handling in the editor.
…s and paddings for improved layout, and modify row label to use styleClass for better representation.
…ith validation, integrate form submission handling, and update layout for improved user interaction. Adjust styles for right sidebar and contentlet tools to support new features.
…ing state during submission, and update submit button behavior for improved user experience.
…order component for improved row management, enhancing drag-and-drop functionality and simplifying layout handling.
BREAKING CHANGE: Major feature consolidation to eliminate duplication Phase 6.1: Merge withPageAsset into withClient - Eliminated withPageAsset.ts entirely (9 duplicate computed signals removed) - Moved all pageAsset computeds (pageData, pageSite, pageContainers, etc.) into withClient - withClient is now the single source of truth for both client config AND page data - Updated all imports from withPageAsset to withClient - Renamed 'layout()' to 'pageLayout()' in dot-row-reorder component Benefits: - Eliminated redundant feature managing same data - Single feature owns pageAssetResponse and its computed views - Clearer ownership: withClient = client config + page data extraction Phase 6.2: Move editor computeds from withPageContext to withEditor - Moved 5 editor permission/capability computeds to withEditor: - editorCanEditContent - editorCanEditLayout - editorCanEditStyles - editorEnableInlineEdit - editorHasAccessToEditMode - withPageContext now contains ONLY cross-cutting concerns (view, workflow, page metadata) - withEditor owns ALL editor-related logic (UI state + permissions + capabilities) Benefits: - Clear domain ownership: all editor logic in one feature - withPageContext reduced to truly shared cross-cutting concerns - No more editor logic scattered across multiple features Impact: - Store composition order unchanged - TypeScript types properly inferred through feature composition - All components continue to work with domain-prefixed APIs Files changed: 7 files - Deleted: withPageAsset.ts - Modified: withClient.ts, withEditor.ts, withPageContext.ts, withStoreComputed.ts - Modified: dot-uve.store.ts, models.ts, dot-row-reorder.component.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix eslint import grouping rules - Remove empty lines within import groups - Add spacing between different import groups Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove empty line between related imports from same directory - All imports from '../../../' grouped together without spacing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add empty line between '../../../utils' and '../../models' import groups Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Removed editor-specific computed properties from withPageContext, consolidating them into withEditor for clearer domain ownership. - Updated withPageContext to focus solely on cross-cutting concerns, including view mode, workflow status, and page metadata. - Enhanced type safety and maintainability by ensuring all editor-related logic is centralized in withEditor. Files modified: withPageContext.ts, withEditor.ts
BREAKING CHANGE: Remove withUVEToolbar feature (100% duplicate) Analysis revealed withUVEToolbar is: - 100% duplicate of withView (same state, computeds, methods) - Not imported anywhere in the store - Not used by any components - Pure dead code Evidence: - withView already provides all toolbar functionality - Store composition uses withView, not withUVEToolbar - Zero references in codebase This is Phase 1 of feature consolidation to reduce 15 features → 7. Next phases: - Phase 2: Consolidate lock concerns into withWorkflow - Phase 3: Merge withPageContext into withClient → withPageData - Phase 4: Merge withViewZoom into withView - Phase 5: Merge withSave + withLayout into withLoad Files deleted: - features/editor/toolbar/withUVEToolbar.ts Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Moved workflowIsPageLocked computed from withPageContext and withEditor to withWorkflow (single source of truth) - Moved systemIsLockFeatureEnabled computed from withPageContext to withWorkflow - Moved lock UI props ($unlockButton, $workflowLockOptions) from withView to withWorkflow - Merged withLock.ts methods (lockPage, unlockPage, workflowToggleLock) into withWorkflow - Deleted withLock.ts file (fully consolidated) - Updated store composition: withWorkflow now comes before withPageContext - Fixed circular dependency with optional pageReload parameter - Removed duplicate lock computeds from withPageContext and withEditor - Updated withEditor to use WorkflowComputed interface from withWorkflow - Removed workflowIsPageLocked from withView dependencies - Fixed import ordering in withWorkflow.ts, withWorkflow.spec.ts, wihtLayout.spec.ts - Removed unused UVE_STATUS, ToggleLockOptions, UnlockOptions imports from withView.ts This eliminates duplicate lock logic across 4 features (withPageContext, withEditor, withView, withLock) and consolidates all lock concerns in a single place.
- Fixed circular dependency between withWorkflow and withLoad by accessing pageReload directly from store at runtime using (store as any).pageReload?.() - Removed WithWorkflowDeps.pageReload parameter to avoid compile-time type errors - Added canLock property to ToggleLockOptions interface (was missing after consolidation) - Updated $workflowLockOptions computed to include canLock: page.canLock ?? false - Build now succeeds with no TypeScript errors This fixes the TypeScript compilation errors introduced in Phase 6.2.
- Moved all page-related computeds from withPageContext to withClient - pageLanguageId, pageLanguage, pageURI, pageVariantId - pageTranslateProps, pageFriendlyParams - Moved viewMode to withView (proper domain ownership) - Removed duplicate viewMode from withEditor - Updated PageAssetComputed interface with merged properties - Deleted withPageContext.ts (87 lines eliminated) - Removed withPageContext from store composition - Updated withView to use PageAssetComputed props (removed deps parameter) - Updated withEditor to use ViewComputed interface for viewMode - Fixed circular references by using local computed values This consolidates all page-related computeds in one place (withClient) and eliminates duplication between withPageContext and withEditor (viewMode was computed in both).
Renamed withClient feature to withPage to better reflect its purpose: - 90% page data access, 10% page loading configuration - Directory: features/client → features/page - File: withClient.ts → withPage.ts - Function: withClient() → withPage() Renamed interfaces for clarity: - ClientConfigState → PageLoadingConfigState (with detailed JSDoc) - WithClientMethods → WithPageMethods Updated all imports across codebase: - dot-uve.store.ts - withEditor.ts, withView.ts, withWorkflow.ts - withStoreComputed.ts - All spec files (withLayout, withLoad, withWorkflow) Documentation improvements: - Added comprehensive JSDoc to PageLoadingConfigState - Clarified that client config is "page loading configuration" - Updated store composition comments Build verified successful - all TypeScript compilation passes.
…-prefixed state - Add withUve feature managing uveStatus, uveIsEnterprise, uveCurrentUser - Update UVEState interface with flat state structure and domain prefixes - Migrate property names: status→uveStatus, isEnterprise→uveIsEnterprise, currentUser→uveCurrentUser, errorCode→pageErrorCode, languages→pageLanguages, experiment→pageExperiment, viewOrientation→viewDeviceOrientation - Update 15+ component and feature files with new property references - Add comprehensive unit tests for withUve (13 tests passing) - Fix TypeScript types: remove any types, properly type dependencies Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix empty interface linting errors in withView and withWorkflow - Remove unused imports (patchState, Signal, byTestId, DotDevice) - Fix parsing error in edit-ema-editor.component.spec.ts (double quotes) - Replace any types with proper TypeScript types in store features - Add DotLanguage import for proper pageLanguage typing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Create withHistory feature as reusable composition utility - Generic history tracking with configurable max entries (default: 50) - Deep cloning support with structuredClone (fallback to JSON) - Computed signals: canUndo, canRedo, historyLength, isAtStart, isAtEnd - Methods: addToHistory, undo, redo, goTo, clearHistory, getHistory - Mock structuredClone in tests for Node.js compatibility - Update withPage to compose with withHistory - Replace withTimeMachine with withHistory (same interface) - Track pageAssetResponse changes for optimistic updates - Support undo/redo for style editor, layout changes - Update store dependencies to use addToHistory method - Fix withLoad and withSave dependencies - Update dot-uve-style-editor-form component - Build successful, all tests passing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Document why 'any' is appropriate for the store parameter in selector - Keep the any localized to the generic config interface - Avoids forcing callers to cast or complicating the API with extra generics - Add eslint-disable with detailed justification Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Consolidate all view concerns (device preview, SEO preview, zoom controls, view mode) into single withView feature - Remove redundant boolean flags (viewIsEditState, viewIsPreviewModeActive) in favor of viewMode enum - Add zoom functionality directly to withView with flat state structure - Add comprehensive JSDoc documenting feature responsibilities - Delete withViewZoom feature and zoom/ directory - Update components to use viewMode() === UVE_MODE.EDIT pattern - Maintain flat state structure following NgRx Signal Store best practices - Fix lint issues in withHistory.spec.ts (remove unused imports, fix non-null assertions) Modified: - models.ts: Remove redundant flags, add flat zoom properties - dot-uve.store.ts: Remove withViewZoom from composition - withView.ts: Add zoom computeds and methods with documentation - withEditor.ts: Update to use viewMode instead of viewIsEditState - edit-ema-editor.component.ts: Update computed signals to use viewMode - withHistory.spec.ts: Fix lint warnings (remove patchState import, fix non-null assertions) Deleted: - store/features/zoom/withZoom.ts - store/features/zoom/models.ts - store/features/zoom/ directory Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Consolidate all backend API interactions into single withPageApi feature - Merge withLoad and withSave features for better organization - All page loading and saving operations now in one place - Clearer ownership of backend communication concerns - Consistent error handling and uveStatus orchestration across all API calls New Feature - withPageApi: - pageLoad() - Load page with all related data (languages, experiments, license, user) - pageReload() - Refresh current page with optional param updates - pageUpdateParams() - Update page parameters (language, variant, etc.) - editorSave() - Save page content (containers) - updateRows() - Save page layout (update rows) - saveStyleEditor() - Save style properties with optimistic updates and rollback Modified: - dot-uve.store.ts: Replace withLoad and withSave with withPageApi - Added comprehensive JSDoc documenting feature responsibilities - Organized dependencies into logical groups (client config, workflow, request metadata, page management, history) Deleted: - store/features/load/withLoad.ts - store/features/load/ directory - store/features/editor/save/withSave.ts - store/features/editor/save/ directory Benefits: - Single responsibility for all backend interactions - Reduced feature count (11 → 10 features) - Clear orchestration of uveStatus during async operations - Centralized error handling with rollback support - Type safety via explicit WithPageApiDeps interface Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add TODO comment explaining why page accessors are passed as dependencies - Document potential solutions to revisit: reorder composition, props type assertion, type assertion - Current approach is necessary due to TypeScript type inference limitations with feature composition order - No functional changes, build still successful Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ation - Add comprehensive JSDoc to withEditor explaining all responsibilities - Add comprehensive JSDoc to withWorkflow explaining lock management - Document why pageReload() uses type assertion in withWorkflow - Clarify feature composition order dependency (withWorkflow before withPageApi) - Improve inline comments for better code clarity - No functional changes - documentation and comment improvements only withEditor improvements: - Document responsibilities: UI state, panel preferences, edit capabilities, inline editing - Document dependencies: UVEState, PageAssetComputed, WorkflowComputed, ViewComputed - Already uses correct naming from previous phases (viewMode, uveIsEnterprise) withWorkflow improvements: - Document responsibilities: workflow actions, lock/unlock, lock state computeds - Explain type assertion necessity for pageReload() access - Note runtime safety despite compile-time limitation - Already uses correct naming (uveCurrentUser, pageErrorCode, uveStatus) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add comprehensive JSDoc documenting feature composition order in main store - Document dependencies for each feature in composition chain - Explain circular dependency between withWorkflow and withPageApi - Add inline comments numbering each feature in composition order - Verify current order is optimal given dependency constraints Feature Composition Order (11 features): 1. withState - Base state 2. withUve - Global system state 3. withFlags - Feature flags 4. withPage - Page data + history 5. withTrack - Analytics 6. withWorkflow - Workflow + lock 7. withMethods - Helper methods 8. withLayout - Layout operations 9. withView - View modes + zoom 10. withEditor - Editor UI 11. withPageApi - Backend API (must be last) Circular Dependency Analysis: - withWorkflow needs pageReload() from withPageApi (type assertion) - withPageApi needs workflowFetch() from withWorkflow (dependency injection) - Current order is optimal - cannot be reordered without breaking dependencies - Type assertion in withWorkflow is necessary and acceptable solution No functional changes - documentation only Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update test files to use new property names from Phase 1: - Replace store.currentUser() with store.uveCurrentUser() - Replace store.status() with store.uveStatus() - Replace store.languages() with store.pageLanguages() Files updated: - withView.spec.ts: Update uveCurrentUser reference - dot-uve.store.integration.spec.ts: Update uveStatus and pageLanguages references Note: Service method calls like dotLicenseService.isEnterprise() remain unchanged as they are not store properties. Verified: No remaining old naming conventions in component/test files. All occurrences of store.status() in palette store are for local store, not UVE store, so they remain unchanged. Phase 7 complete - Component updates done.
Create comprehensive README.md explaining UVE Store architecture: Core Principles: - Flat state structure (NgRx Signal Store best practice) - Domain prefixes for clear ownership (uve*, page*, editor*, view*, workflow*) - Single responsibility per feature - Composition patterns for reusability Key Documentation: - Feature composition order with dependencies explained - Circular dependency between withWorkflow ↔ withPageApi documented - Scaling rules for adding new state and creating features - Common patterns for accessing state, updating, and checking permissions - Property naming conventions Focused on: - WHY the architecture is designed this way - RULES for scaling the store safely - PATTERNS for common operations Phase 8 complete - Documentation done (~150 lines, focused on architecture).
There was a problem hiding this comment.
Pull request overview
This PR introduces a real-time canvas for the Universal Visual Editor (UVE) with zoom controls, a right sidebar for content editing, and comprehensive store refactoring. The changes establish a cleaner architecture with flat state structure, domain-prefixed properties, and improved separation of concerns.
Changes:
- Implements zoom functionality with controls and gesture support for the UVE canvas
- Adds a right sidebar with tabbed interface for content editing and style editor
- Refactors UVE store to use flat state structure with domain prefixes (
uve*,page*,editor*,view*,workflow*) - Introduces new presentational components following container/presentational pattern
- Removes deprecated store features and consolidates state management
Reviewed changes
Copilot reviewed 101 out of 130 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| withUVEToolbar.ts | File deleted - toolbar state moved to withView feature |
| withSave.ts | File deleted - save functionality consolidated into withPageApi |
| withClient.ts | File deleted - client configuration moved to withPage feature |
| dot-uve.store.ts | Complete refactoring with flat state structure and domain-prefixed properties |
| README.md | Added comprehensive architecture documentation with 200+ lines |
| models.ts | Removed deprecated interfaces (UVEToolbarProps, EditorProps, UnlockOptions) and updated palette tabs enum |
| shared/models.ts | Removed UnlockOptions interface and showBanner/showOverlay from ToggleLockOptions |
| mocks.ts | Updated page mock properties from isLocked/lockedByUser to locked/lockedBy/lockedByName |
| edit-ema-editor.component.scss | Added zoom canvas layout with grid system and right sidebar wrapper |
| edit-ema-editor.component.html | Complete template refactor with zoom controls, browser toolbar, and right sidebar tabs |
| dot-uve-zoom-controls component | New component for zoom in/out/reset controls |
| dot-uve-iframe component | New component extracted from editor with height reporting and traditional page support |
| dot-uve-contentlet-quick-edit component | New presentational component for inline contentlet editing with dynamic form generation |
| dot-uve-palette.component.ts | Refactored to use local signalState for tab management instead of global store |
| lib.routes.ts | Removed DotEditPageResolver and DotPageStateService, added UVEStore and dependencies as providers |
Comments suppressed due to low confidence (1)
core-web/libs/portlets/edit-ema/portlet/src/lib/store/dot-uve.store.ts:1
- The comment mentions a circular dependency between withWorkflow and withPageApi but doesn't provide a clear reference to where this is documented or how to resolve it safely. Consider adding a reference to related documentation or tracking issue for future resolution.
import { signalStore, withFeature, withMethods, withState } from '@ngrx/signals';
| palette: { | ||
| open: boolean; | ||
| currentTab: UVE_PALETTE_TABS; | ||
| // currentTab removed - now managed locally in DotUvePaletteComponent |
There was a problem hiding this comment.
The comment on line 28 should be updated to reflect that currentTab is now managed as local state using signalState in DotUvePaletteComponent, rather than being removed entirely. The current wording might suggest the feature was deleted rather than relocated.
| // currentTab removed - now managed locally in DotUvePaletteComponent | |
| // currentTab is now managed as local state via signalState in DotUvePaletteComponent |
| STYLE_EDITOR = 4, | ||
| LAYERS = 3 |
There was a problem hiding this comment.
The enum values for STYLE_EDITOR (4) and LAYERS (3) are out of order. For maintainability and clarity, enum values should be sequential. Consider reordering to have LAYERS = 3 followed by STYLE_EDITOR = 4, or renumbering to maintain sequential order.
| STYLE_EDITOR = 4, | |
| LAYERS = 3 | |
| LAYERS = 3, | |
| STYLE_EDITOR = 4 |
| lockedBy: string; | ||
| canLock: boolean; | ||
| isLockedByCurrentUser: boolean; |
There was a problem hiding this comment.
The ToggleLockOptions interface is missing documentation for the lockedBy, canLock, and isLockedByCurrentUser properties. Adding JSDoc comments would improve code maintainability and developer experience.
| // Adjust coordinates for zoom level | ||
| const adjustedHeight = isEmpty ? targetRect.height / this.zoomLevel : 3; |
There was a problem hiding this comment.
The magic number 3 represents the height in pixels for the drop indicator. This should be extracted as a named constant (e.g., DROP_INDICATOR_HEIGHT_PX = 3) to improve code readability and maintainability.
| const lang = this.language(); | ||
| if (!lang) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The null check added for lang on lines 38-41 is a good defensive programming practice. However, similar null safety should also be verified in the ngAfterViewInit and onChange methods to ensure consistency across the component lifecycle.
| }; | ||
| readonly $disableOrientation = computed( | ||
| () => this.#store.device()?.inode === 'default' || this.#store.socialMedia() | ||
| () => this.state().currentDevice?.inode === 'default' || this.state().currentSocialMedia !== null |
There was a problem hiding this comment.
The magic string 'default' is used to check the device inode. This should be extracted as a named constant (e.g., DEFAULT_DEVICE_INODE = 'default') to improve maintainability and prevent typos in future comparisons.
|
|
||
| protected readonly DotCMSClazzes = DotCMSClazzes; | ||
|
|
||
| constructor() { | ||
| // Build form when data changes | ||
| effect(() => { | ||
| const { fields, contentlet } = this.data(); | ||
|
|
||
| if (!fields || fields.length === 0) { | ||
| this.contentletForm.set(null); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
The effect rebuilds the form on every data() change. Consider adding a check to compare the current contentlet identifier with the previous one to avoid unnecessary form rebuilds when only non-form-related data changes.
| protected readonly DotCMSClazzes = DotCMSClazzes; | |
| constructor() { | |
| // Build form when data changes | |
| effect(() => { | |
| const { fields, contentlet } = this.data(); | |
| if (!fields || fields.length === 0) { | |
| this.contentletForm.set(null); | |
| return; | |
| } | |
| private readonly previousContentletInode = signal<string | null>(null); | |
| protected readonly DotCMSClazzes = DotCMSClazzes; | |
| constructor() { | |
| // Build form when data changes, but avoid unnecessary rebuilds | |
| effect(() => { | |
| const { fields, contentlet } = this.data(); | |
| const currentInode = contentlet?.inode ?? null; | |
| const previousInode = this.previousContentletInode(); | |
| if (!fields || fields.length === 0 || !contentlet) { | |
| this.contentletForm.set(null); | |
| this.previousContentletInode.set(null); | |
| return; | |
| } | |
| // If the contentlet identifier hasn't changed and a form already exists, | |
| // skip rebuilding the form. | |
| if (previousInode === currentInode && this.contentletForm()) { | |
| return; | |
| } | |
| this.previousContentletInode.set(currentInode); |
| <!-- TODO: Needs to translate the label --> | ||
| <p-button | ||
| type="button" | ||
| label="Cancel" | ||
| [outlined]="true" | ||
| (click)="handleCancel()" | ||
| [disabled]="loading()"></p-button> | ||
| <p-button | ||
| type="submit" | ||
| label="Save" |
There was a problem hiding this comment.
The TODO comment on line 85 indicates missing translation for button labels. This should be tracked and resolved to maintain consistency with the rest of the application's i18n implementation.
| <!-- TODO: Needs to translate the label --> | |
| <p-button | |
| type="button" | |
| label="Cancel" | |
| [outlined]="true" | |
| (click)="handleCancel()" | |
| [disabled]="loading()"></p-button> | |
| <p-button | |
| type="submit" | |
| label="Save" | |
| <p-button | |
| type="button" | |
| [label]="'dot.common.cancel' | dm" | |
| [outlined]="true" | |
| (click)="handleCancel()" | |
| [disabled]="loading()"></p-button> | |
| <p-button | |
| type="submit" | |
| [label]="'dot.common.save' | dm" |
| ); | ||
| } | ||
|
|
||
| private maybeHandleIframeHeightMessage(event: MessageEvent, onClampScroll: () => void): boolean { |
There was a problem hiding this comment.
The method name maybeHandleIframeHeightMessage uses "maybe" which is ambiguous. Consider renaming to tryHandleIframeHeightMessage or handleIframeHeightMessageIfValid to better convey that this is a conditional handler that returns success status.
| export function isQuickEditSupportedField(clazz: string): clazz is QuickEditFieldClass { | ||
| return QUICK_EDIT_SUPPORTED_FIELDS.includes(clazz as QuickEditFieldClass); | ||
| } |
There was a problem hiding this comment.
The type guard isQuickEditSupportedField casts clazz to QuickEditFieldClass before checking inclusion. This defeats the purpose of the type guard. The cast should be removed and the return type should properly narrow the type based on the runtime check.
Security: - Force regenerate yarn.lock to match package.json Angular 20.3.16 - Addresses Semgrep CVE-2026-22610 (XSS via SVG script href attributes) Code Quality (10 Copilot findings): - Update comment clarity for currentTab state management - Reorder enum values for sequential ordering (LAYERS=3, STYLE_EDITOR=4) - Add JSDoc documentation for ToggleLockOptions properties - Extract magic numbers/strings to named constants (DROP_INDICATOR_HEIGHT_PX, DEFAULT_DEVICE_INODE) - Add null safety checks in language selector lifecycle methods - Optimize effect to avoid unnecessary form rebuilds - Add i18n translations for Cancel/Save button labels - Rename ambiguous method tryHandleIframeHeightMessage - Fix type guard in isQuickEditSupportedField to properly narrow types No functional changes - addresses security and maintainability improvements Resolves all review findings in PR #34173 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Remove 'Phase X.Y:' prefixes from describe() blocks in specs - Remove 'Phase X.Y:' from inline comments in component and store files - Keep semantic meaning of comments while removing development phase markers - Documentation files (*.md) intentionally not modified No functional changes - code cleanup only
| // Check if we already have page data loaded with matching params | ||
| // This prevents reloading when navigating between child routes (content <-> layout) | ||
| const currentPageParams = this.uveStore.pageParams(); | ||
| const hasPageData = !!this.uveStore.pageAsset()?.page; | ||
| const paramsMatch = currentPageParams && | ||
| currentPageParams.url === params.url && | ||
| currentPageParams.language_id === params.language_id && | ||
| currentPageParams.mode === params.mode; | ||
|
|
||
| // Only load if we don't have data or params have changed | ||
| if (!hasPageData || !paramsMatch) { | ||
| this.uveStore.pageLoad(params); | ||
| } |
There was a problem hiding this comment.
I need to test and review this.
| case NG_CUSTOM_EVENTS.UPDATE_WORKFLOW_ACTION: { | ||
| const pageAPIResponse = this.uveStore.pageAPIResponse(); | ||
| this.uveStore.getWorkflowActions(pageAPIResponse.page.inode); | ||
| // Removed pageAPIResponse - use normalized accessors |
There was a problem hiding this comment.
This comment is not needed.
There was a problem hiding this comment.
This needs permissions, what about if the user don't have permissions to edit a piece of content? how that works right now? however it works we need to do the same here.
There was a problem hiding this comment.
Also how do we handle required field here?
| * @if ($rightSidebarOpen()) { | ||
| * <dot-uve-contentlet-quick-edit | ||
| * [data]="$contentletEditData()" | ||
| * [loading]="$isSubmitting()" |
There was a problem hiding this comment.
I don't like an architecture where a form component receives a "loading" input.
| private readonly contentletForm = signal<FormGroup | null>(null); | ||
| protected readonly $contentletForm = computed(() => this.contentletForm()); |
There was a problem hiding this comment.
Why do we need two here? we might need signal state or something.
There was a problem hiding this comment.
Oops, needs to fix this.
| @@ -7,6 +7,41 @@ import Footer from "@/components/footer/Footer"; | |||
| import Header from "@/components/header/Header"; | |||
| import { ACTIVITY_SCHEMA, BANNER_SCHEMA } from "@/utils/styleEditorSchemas"; | |||
|
|
|||
| import { useEffect } from 'react'; | |||
|
|
|||
| export function IframeHeightBridge() { | |||
There was a problem hiding this comment.
This needs to go to the SDK.
Proposed Changes
Checklist
Additional Info
** any additional useful context or info **
Screenshots