Skip to content

Conversation

@GiangNT95
Copy link
Contributor

Feature: CMS-47083

@GiangNT95 GiangNT95 requested review from TRomesh and exacs November 27, 2025 07:35
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Feature: CMS-47083
@GiangNT95 GiangNT95 force-pushed the feature/CMS-47083-JS-SDK-Add-type-Infer-for-displaySettings branch from aae4148 to 37d442f Compare November 28, 2025 09:26
@GiangNT95 GiangNT95 marked this pull request as ready for review December 1, 2025 02:21
@GiangNT95 GiangNT95 enabled auto-merge December 1, 2025 02:21
});

if (!Component) {
console.log(
Copy link
Contributor

Choose a reason for hiding this comment

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

This was removed my a previous PR. Maybe rebase the branch again?

settings: DisplaySettingsType[] | null | undefined,
templateSettings: Record<string, { editor: string; choices: Record<string, any> }>
): Record<string, string> {
if (!settings || settings.length === 0) return {};
Copy link
Contributor

Choose a reason for hiding this comment

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

We should return undefined instead of an empty {}.


const parsedDisplaySettings = template
? parseDisplaySettings(node.displaySettings, template.settings)
: {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Again here. We should return undefined if there is no displaySettings returned

// Assign the value to the key in the result object
result[item.key] = item.value;
}
settings.forEach(s => {
Copy link
Contributor

@TRomesh TRomesh Dec 1, 2025

Choose a reason for hiding this comment

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

On the second thought we might not need to do this while parsing. Lets stick to the older way where we just did something like this

  for (const item of displaySettings) {
    // Assign the value to the key in the result object
    result[item.key] = item.value;
  }

return undefined; // Return undefined if displaySettings is not provided
}
settings: DisplaySettingsType[] | null | undefined,
templateSettings: Record<string, { editor: string; choices: Record<string, any> }>
Copy link
Contributor

Choose a reason for hiding this comment

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

lets stick to old way where we take just settings (a displaySettings)

Feature: CMS-47083
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.

4 participants