Skip to content

feat(uve): Add validation of FF TOGGLE_LOCK on different pageViews#35747

Queued
KevinDavilaDotCMS wants to merge 2 commits into
mainfrom
35743-uve-lock-overlay-covers-the-iframe-in-preview-and-live-modes-when-feature_flag_uve_toggle_lock-is-enabled
Queued

feat(uve): Add validation of FF TOGGLE_LOCK on different pageViews#35747
KevinDavilaDotCMS wants to merge 2 commits into
mainfrom
35743-uve-lock-overlay-covers-the-iframe-in-preview-and-live-modes-when-feature_flag_uve_toggle_lock-is-enabled

Conversation

@KevinDavilaDotCMS
Copy link
Copy Markdown
Member

@KevinDavilaDotCMS KevinDavilaDotCMS commented May 19, 2026

2026-05-19.11-59-42.mov

Screenshots

Original Updated
** original screenshot ** ** updated screenshot **

This PR fixes: #35743

…mode

Introduces a new unit test to verify that the lock overlay is not displayed when the view mode is set to PREVIEW. This change enhances the test coverage for the EditEmaEditorComponent, ensuring correct behavior under different view modes. Additionally, the import statement for the DotUveIframeComponent has been restored to maintain functionality.

This PR improves the reliability of the component's behavior in various scenarios.
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 19, 2026

Claude finished @KevinDavilaDotCMS's task in 1m 23s —— View job


Review

The fix is correct: early-returning false from $showLockOverlay when viewMode !== UVE_MODE.EDIT (edit-ema-editor.component.ts:356-359) correctly suppresses the overlay in PREVIEW, LIVE, and UNKNOWN modes, which is the bug from #35743. The lock UI only makes sense in EDIT — gating at the computed level (vs. in the template) keeps the logic centralized. Two human approvals already.

A few observations worth a second look — none blocking:

  • Spec coverage is narrow. The new test (edit-ema-editor.component.spec.ts:881-892) only validates UVE_MODE.PREVIEW with the feature flag disabled. The new branch also affects LIVE and UNKNOWN, and the more dangerous regression — feature flag enabled + page not locked — would have returned true under the old logic. An it.each([PREVIEW, LIVE, UNKNOWN]) paired with both flag states would have been a tighter regression net for the same line count.

  • Flag in the new test is irrelevant noise. flags: { FEATURE_FLAG_UVE_TOGGLE_LOCK: false } at line 887 has no effect because the early-return at line 357 short-circuits before the flag is read. Either drop it or flip it to true to also exercise the enabled-path regression (which is what the issue title actually calls out).

  • Test placement. The new it sits between the lockedByAnotherUser mock declaration and the with feature flag enabled describe. Moving it under its own describe('when view mode is not EDIT', ...) would group the non-EDIT cases and read more clearly alongside the existing flag-on / flag-off branches.

  • Lint-fix commit is import-order churn. be5ab1a8 reverts the import order back to what b4a25dc0 already had after the rebase — cosmetic, fine to leave, just noting it shows up in the history.

  • Defensive ... ?? {} on pageParams. ...(store.pageParams() ?? {}) at line 884 is defensive against an undefined pageParams that the default test setup (line 490) already populates with mode: UVE_MODE.EDIT. Not wrong, just unnecessary.

No security, correctness, or design concerns. Clean to merge.
Branch

@KevinDavilaDotCMS KevinDavilaDotCMS added this pull request to the merge queue May 20, 2026
Any commits made after this event will not be merged.
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.

UVE lock overlay covers the iframe in Preview and Live modes when FEATURE_FLAG_UVE_TOGGLE_LOCK is enabled

3 participants