-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(dashboard): restore email editor 'for' block #7483
base: next
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for dev-web-novu ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for dashboard-v2-novu-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
private async traverseAndProcessNodes( | ||
node: TipTapNode, | ||
parentNode?: TipTapNode | ||
): Promise<TipTapNode> { | ||
if (node.content) { | ||
const processedContent: TipTapNode[] = []; | ||
for (const innerNode of node.content) { | ||
const processed = await this.processShowAndForControls(variables, innerNode, parentNode); | ||
if (processed) { | ||
processedContent.push(processed); | ||
variables: Record<string, unknown>, | ||
parent?: TipTapNode | ||
): Promise<void> { | ||
const queue: Array<{ node: TipTapNode; parent?: TipTapNode }> = [{ node, parent }]; | ||
|
||
while (queue.length > 0) { | ||
const current = queue.shift()!; | ||
await this.processNode(current.node, variables, current.parent); | ||
|
||
if (current.node.content) { | ||
for (const childNode of current.node.content) { | ||
queue.push({ node: childNode, parent: current.node }); | ||
} | ||
} | ||
node.content = processedContent; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworked the previously recursive function to something easier to read using queue.
private addIndexesToPlaceholders(nodes: TipTapNode[], iterablePath: string, index: number): TipTapNode[] { | ||
return nodes.map((node) => { | ||
const newNode: TipTapNode = { ...node }; | ||
|
||
const parsedShowIfValue = await parseLiquid(showIfKey as string, variables); | ||
const showIfValueBoolean = | ||
typeof parsedShowIfValue === 'boolean' ? parsedShowIfValue : this.stringToBoolean(parsedShowIfValue); | ||
if (this.isAVariableNode(newNode)) { | ||
const nodePlaceholder = newNode.text as string; | ||
|
||
/** | ||
* example: | ||
* iterablePath = payload.comments | ||
* nodePlaceholder = {{ payload.comments.author }} | ||
* text = {{ payload.comments[0].author }} | ||
*/ | ||
newNode.text = nodePlaceholder.replace(iterablePath, `${iterablePath}[${index}]`); | ||
newNode.type = 'text'; // set 'variable' to 'text' to for Liquid to recognize it | ||
} else if (newNode.content) { | ||
newNode.content = this.addIndexesToPlaceholders(newNode.content, iterablePath, index); | ||
} | ||
|
||
if (!showIfValueBoolean) { | ||
this.removeNodeFromParent(node, parentNode); | ||
} else { | ||
delete node.attrs[MailyAttrsEnum.SHOW_IF_KEY]; | ||
} | ||
return newNode; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of replacing for loop values directly in this usecase as it was before, we create just placeholders {{ payload.comments[0].author }}
and leave the actual value substitution for Liquid parser down the line
@@ -157,27 +214,7 @@ export class ExpandEmailEditorSchemaUsecase { | |||
} | |||
|
|||
private isAVariableNode(newNode: TipTapNode): newNode is TipTapNode & { attrs: { id: string } } { | |||
return newNode.type === 'payloadValue' && newNode.attrs?.id !== undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea what is payloadValue
but I was not able to find it anywhere in the TipTapNode
@@ -132,10 +132,13 @@ export const Maily = (props: MailyProps) => { | |||
return dedupAndSortVariables(filteredVariables, queryWithoutSuffix); | |||
} | |||
|
|||
const iterableName = editor?.getAttributes('for')?.each; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for now the iterable name is the full path, e.g. - we can polish this in separate PR, it was too difficult for me to change all that at once
for each "payload.comments"
{{payload.comments.id}}
What changed? Why was the change needed?
{{payload.comments}}
, item is{{payload.comments.id}}
High-level overview of how the parsing flow goes so we can reference it later when refactoring:
for-loop.mov