Skip to content

Conversation

Amitkanswal
Copy link
Contributor

Changes- support for new JSON RTE plugin

@Amitkanswal Amitkanswal requested a review from a team June 25, 2025 07:02
@Amitkanswal Amitkanswal requested a review from a team as a code owner June 25, 2025 07:02
@Amitkanswal Amitkanswal requested a review from rijil-tr June 26, 2025 19:15
@Amitkanswal Amitkanswal marked this pull request as draft July 7, 2025 09:02
return {
__isPluginBuilder__: true,
version,
plugins: (context: InitializationData, rte: IRteParam) => {

Choose a reason for hiding this comment

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

Why do we need to use the broader type here when we know we are getting RTE specific initialization data?

this._config.elementType = elementType;
return this;
}
render(renderFn: (element: React.ReactElement, attrs: { [key: string]: any }, path: number[], rte: IRteParam) => React.ReactElement): PluginBuilder {

Choose a reason for hiding this comment

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

Below is the signature of existing render function

 render?: (...params: any) => ReactElement

Why are we introducing additional arguments now? Why these issues not occurring with existing plugins?

const plugins = async (context: InitializationData, rte: IRteParam) => {
try {
const sdk = new UiLocation(context);
console.log("sdk", sdk);

Choose a reason for hiding this comment

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

Remove console logs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

| "GCP_NA"
| string;

export type Extension = {

Choose a reason for hiding this comment

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

Why are we adding this now?

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 will resolve this

this.modal = new Modal();

this.region = formatAppRegion(initializationData.region);
this.region = formatAppRegion(initializationData.region as Region);

Choose a reason for hiding this comment

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

Why are we using Region here? region is of type RegionType right?

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 is the older version PR changes for region is present in develop branch

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 will create a new pr and close this one

}

export function formatAppRegion(region: string): RegionType {
return region ?? Region.UNKNOWN;

Choose a reason for hiding this comment

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

Why this change is needed?

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.

5 participants