Skip to content

Vp 444 by default bug fix#558

Merged
contentstackMridul merged 3 commits intoVP-444from
VP-444_by_default_bug_fix
Mar 10, 2026
Merged

Vp 444 by default bug fix#558
contentstackMridul merged 3 commits intoVP-444from
VP-444_by_default_bug_fix

Conversation

@contentstackMridul
Copy link
Contributor

Vp 444 by default bug fix

@contentstackMridul contentstackMridul requested a review from a team as a code owner March 10, 2026 10:21
editInVisualBuilderButton: IConfigEditInVisualBuilderButton;
mode: ILivePreviewMode;
enableLivePreviewOutsideIframe: boolean; // default: false
enableLivePreviewOutsideIframe: boolean | undefined; // default: undefined
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not this?

Suggested change
enableLivePreviewOutsideIframe: boolean | undefined; // default: undefined
enableLivePreviewOutsideIframe?: boolean;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially considered enableLivePreviewOutsideIframe?: boolean, but with exactOptionalPropertyTypes: true that type does not allow explicitly assigning undefined when the key is present in object literals.
In our defaults, we intentionally keep the key present as undefined so the config shape stays consistent and key validation via Config.set continues to work.

Config.set(
"enableLivePreviewOutsideIframe",
initData.enableLivePreviewOutsideIframe ??
stackSdk.live_preview?.enableLivePreviewOutsideIframe ??
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when and where this stackSdk?.. value is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stack SDK is directly coming from the user as SDK instance, but the value of enableLivePreviewOutsideIframe will not be present in that.
I’ve removed stackSdk.live_preview?.enableLivePreviewOutsideIframe from the merge chain and now resolve it only from:

  1. initData.enableLivePreviewOutsideIframe (explicit user input), then
  2. existing config.enableLivePreviewOutsideIframe.
    This keeps behavior explicit and avoids unintended sourcing.

@github-actions
Copy link

github-actions bot commented Mar 10, 2026

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 72.45% 9337 / 12887
🔵 Statements 72.45% 9337 / 12887
🔵 Functions 71.61% 333 / 465
🔵 Branches 83.95% 1167 / 1390
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/configManager/config.default.ts 98.03% 100% 66.66% 98.03% 93-94
src/configManager/handleUserConfig.ts 95.78% 83.78% 100% 95.78% 66, 96-97, 103, 109, 121, 178
src/livePreview/eventManager/postMessageEvent.hooks.ts 80% 92.1% 75% 80% 80-98, 157-158, 180, 193-201
src/types/types.ts 100% 100% 100% 100%
Generated in workflow #754 for commit a9bb94a by the Vitest Coverage Report Action

Copy link
Contributor

@KANE-99 KANE-99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@contentstackMridul contentstackMridul merged commit 97de1fe into VP-444 Mar 10, 2026
8 of 10 checks passed
@contentstackMridul contentstackMridul deleted the VP-444_by_default_bug_fix branch March 10, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants