Skip to content

Updated html markup for CTA card frontend rendering #1442

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.

Sign up for GitHub

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

Merged
merged 16 commits into from
Feb 13, 2025
Merged

Updated html markup for CTA card frontend rendering #1442

Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
57244a7
Updated html markup for CTA card frontend rendering
sanne-san Feb 11, 2025
8eec344
Updated CTA card email rendering
sanne-san Feb 11, 2025
37897fb
Merge branch 'main' into ctacard-rendering
ronaldlangeveld Feb 12, 2025
aece70e
Fixed regex not taking hex color into account
ronaldlangeveld Feb 12, 2025
059302d
Added sponsorLabel to email
ronaldlangeveld Feb 12, 2025
ac64145
Cleanup
ronaldlangeveld Feb 12, 2025
c5c512d
Updated email CTA card rendering
sanne-san Feb 12, 2025
96df53b
Merge branch 'main' into ctacard-rendering
ronaldlangeveld Feb 13, 2025
19c7141
Added image sizes to CTA Card Node
ronaldlangeveld Feb 13, 2025
3eb9140
Added width / height loading to uploader
ronaldlangeveld Feb 13, 2025
3777a50
Added height and width to email rendering
ronaldlangeveld Feb 13, 2025
31dcacd
Merge branch 'main' into ctacard-rendering
ronaldlangeveld Feb 13, 2025
b8ee167
Added cleanup to image change
ronaldlangeveld Feb 13, 2025
d028e71
Added imageWidth and imageHeight to remove media
ronaldlangeveld Feb 13, 2025
8e66d4f
Fixed test
ronaldlangeveld Feb 13, 2025
dc8346b
Cleanup todo comments
ronaldlangeveld Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 120 additions & 32 deletions packages/kg-default-nodes/lib/nodes/call-to-action/calltoaction-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,56 +3,139 @@ import {renderWithVisibility} from '../../utils/visibility';

// TODO - this is a placeholder for the cta card web template
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be removed now 😄

function ctaCardTemplate(dataset) {
const backgroundAccent = dataset.backgroundColor === 'accent' ? 'kg-style-accent' : '';
// Add validation for buttonColor
if (!dataset.buttonColor || !dataset.buttonColor.match(/^[a-zA-Z\d-]+$/)) {
dataset.buttonColor = 'accent';
}

const buttonAccent = dataset.buttonColor === 'accent' ? 'kg-style-accent' : '';
const buttonStyle = dataset.buttonColor !== 'accent' ? `background-color: ${dataset.buttonColor};` : '';
const buttonStyle = dataset.buttonColor === 'accent'
? `style="color: ${dataset.buttonTextColor};"`
: `style="background-color: ${dataset.buttonColor}; color: ${dataset.buttonTextColor};"`;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Sanitize color values before interpolation.

Direct interpolation of user input (buttonColor, buttonTextColor) into style attributes could lead to XSS vulnerabilities. Ensure these values are properly sanitized.

+    const sanitizeColor = (color) => {
+        // Implement color value sanitization
+        return color.replace(/[^#,().\d\w%-]/g, '');
+    };
+
     const buttonStyle = dataset.buttonColor === 'accent' 
-        ? `style="color: ${dataset.buttonTextColor};"` 
-        : `style="background-color: ${dataset.buttonColor}; color: ${dataset.buttonTextColor};"`;
+        ? `style="color: ${sanitizeColor(dataset.buttonTextColor)};"` 
+        : `style="background-color: ${sanitizeColor(dataset.buttonColor)}; color: ${sanitizeColor(dataset.buttonTextColor)};"`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const buttonStyle = dataset.buttonColor === 'accent'
? `style="color: ${dataset.buttonTextColor};"`
: `style="background-color: ${dataset.buttonColor}; color: ${dataset.buttonTextColor};"`;
const sanitizeColor = (color) => {
// Implement color value sanitization
return color.replace(/[^#,().\d\w%-]/g, '');
};
const buttonStyle = dataset.buttonColor === 'accent'
? `style="color: ${sanitizeColor(dataset.buttonTextColor)};"`
: `style="background-color: ${sanitizeColor(dataset.buttonColor)}; color: ${sanitizeColor(dataset.buttonTextColor)};"`;


return `
<div class="cta-card ${backgroundAccent}" data-layout="${dataset.layout}" style="background-color: ${dataset.backgroundColor};">
${dataset.hasImage ? `<img src="${dataset.imageUrl}" alt="CTA Image">` : ''}
<div>
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${dataset.buttonUrl}" class="kg-cta-button ${buttonAccent}"
style="${buttonStyle} color: ${dataset.buttonTextColor};">
${dataset.buttonText}
</a>
` : ''}
<div class="kg-card kg-cta-card kg-cta-bg-${dataset.backgroundColor} kg-cta-${dataset.layout}" data-layout="${dataset.layout}">
${dataset.hasSponsorLabel ? `
<div class="kg-sponsor-label">
<div class="kg-cta-sponsor-label">
Sponsored
</div>
` : ''}
<div class="kg-cta-content">
${dataset.hasImage ? `
<div class="kg-cta-image-container">
<img src="${dataset.imageUrl}" alt="CTA Image">
</div>
` : ''}
<div class="kg-cta-content-inner">
<div class="kg-cta-text">
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${dataset.buttonUrl}" class="kg-cta-button ${buttonAccent}"
${buttonStyle}>
${dataset.buttonText}
</a>
` : ''}
</div>
</div>
</div>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance security and accessibility.

  1. URLs are directly interpolated without sanitization, which could lead to XSS vulnerabilities.
  2. The image alt text is generic and not customizable, which impacts accessibility.
+    const sanitizeUrl = (url) => {
+        try {
+            const parsed = new URL(url);
+            return parsed.toString();
+        } catch {
+            return '#';
+        }
+    };
+
     return `
         <div class="kg-card kg-cta-card kg-cta-bg-${dataset.backgroundColor} kg-cta-${dataset.layout}" data-layout="${dataset.layout}">
             ${dataset.hasSponsorLabel ? `
                 <div class="kg-cta-sponsor-label">
                     Sponsored
                 </div>
             ` : ''}
             <div class="kg-cta-content">
                 ${dataset.hasImage ? `
                     <div class="kg-cta-image-container">
-                        <img src="${dataset.imageUrl}" alt="CTA Image">
+                        <img src="${sanitizeUrl(dataset.imageUrl)}" alt="${dataset.imageAltText || 'CTA Image'}">
                     </div>
                 ` : ''}
                 <div class="kg-cta-content-inner">
                     <div class="kg-cta-text">
                         ${dataset.textValue}
                     </div>
                     ${dataset.showButton ? `
-                        <a href="${dataset.buttonUrl}" class="kg-cta-button ${buttonAccent}"
+                        <a href="${sanitizeUrl(dataset.buttonUrl)}" class="kg-cta-button ${buttonAccent}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div class="kg-card kg-cta-card kg-cta-bg-${dataset.backgroundColor} kg-cta-${dataset.layout}" data-layout="${dataset.layout}">
${dataset.hasSponsorLabel ? `
<div class="kg-sponsor-label">
<div class="kg-cta-sponsor-label">
Sponsored
</div>
` : ''}
<div class="kg-cta-content">
${dataset.hasImage ? `
<div class="kg-cta-image-container">
<img src="${dataset.imageUrl}" alt="CTA Image">
</div>
` : ''}
<div class="kg-cta-content-inner">
<div class="kg-cta-text">
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${dataset.buttonUrl}" class="kg-cta-button ${buttonAccent}"
${buttonStyle}>
${dataset.buttonText}
</a>
` : ''}
</div>
</div>
</div>
const sanitizeUrl = (url) => {
try {
const parsed = new URL(url);
return parsed.toString();
} catch {
return '#';
}
};
return `
<div class="kg-card kg-cta-card kg-cta-bg-${dataset.backgroundColor} kg-cta-${dataset.layout}" data-layout="${dataset.layout}">
${dataset.hasSponsorLabel ? `
<div class="kg-cta-sponsor-label">
Sponsored
</div>
` : ''}
<div class="kg-cta-content">
${dataset.hasImage ? `
<div class="kg-cta-image-container">
<img src="${sanitizeUrl(dataset.imageUrl)}" alt="${dataset.imageAltText || 'CTA Image'}">
</div>
` : ''}
<div class="kg-cta-content-inner">
<div class="kg-cta-text">
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${sanitizeUrl(dataset.buttonUrl)}" class="kg-cta-button ${buttonAccent}"
${buttonStyle}>
${dataset.buttonText}
</a>
` : ''}
</div>
</div>
</div>
`;

`;
}

// TODO - this is a placeholder for the email template
function emailCTATemplate(dataset) {
const buttonStyle = dataset.buttonColor !== 'accent' ? `background-color: ${dataset.buttonColor};` : '';
const buttonStyle = dataset.buttonColor === 'accent'
? `color: ${dataset.buttonTextColor};`
: `background-color: ${dataset.buttonColor}; color: ${dataset.buttonTextColor};`;
const backgroundStyle = `background-color: ${dataset.backgroundColor};`;

const renderContent = () => {
if (dataset.layout === 'minimal') {
return `
<tr>
<td class="kg-cta-content">
<table border="0" cellpadding="0" cellspacing="0" width="100%" class="kg-cta-content-wrapper">
<tr>
${dataset.hasImage ? `
<td class="kg-cta-image-container" width="64">
<img src="${dataset.imageUrl}" alt="CTA Image" class="kg-cta-image" width="64">
</td>
` : ''}
<td class="kg-cta-content-inner">
<div class="kg-cta-text">
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${dataset.buttonUrl}"
class="kg-cta-button ${dataset.buttonColor === 'accent' ? 'kg-style-accent' : ''}"
style="${buttonStyle}">
${dataset.buttonText}
</a>
` : ''}
</td>
</tr>
</table>
</td>
</tr>
`;
}

return `
<tr>
<td class="kg-cta-content">
<table border="0" cellpadding="0" cellspacing="0" width="100%" class="kg-cta-content-wrapper">
${dataset.hasImage ? `
<tr>
<td class="kg-cta-image-container">
<table border="0" cellpadding="0" cellspacing="0" width="100%">
<tr>
<td>
<img src="${dataset.imageUrl}" alt="CTA Image" class="kg-cta-image">
</td>
</tr>
</table>
</td>
</tr>
` : ''}
<tr>
<td class="kg-cta-text">
${dataset.textValue}
</td>
</tr>
${dataset.showButton ? `
<tr>
<td>
<table border="0" cellpadding="0" cellspacing="0" width="100%">
<tr>
<td>
<a href="${dataset.buttonUrl}"
class="kg-cta-button ${dataset.buttonColor === 'accent' ? 'kg-style-accent' : ''}"
style="${buttonStyle}">
${dataset.buttonText}
</a>
</td>
</tr>
</table>
</td>
</tr>
` : ''}
</table>
</td>
</tr>
`;
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor email template for maintainability and security.

The email template has several issues:

  1. Same security vulnerabilities as the web template (unsanitized URLs and colors)
  2. Complex nested table structure with duplicated code between layouts

Consider these improvements:

  1. Extract common table structures into reusable functions:
const renderImageSection = (imageUrl, altText, minimal = false) => `
    <tr>
        <td class="kg-cta-image-container"${minimal ? ' width="64"' : ''}>
            <img src="${sanitizeUrl(imageUrl)}" 
                 alt="${altText || 'CTA Image'}" 
                 class="kg-cta-image"
                 ${minimal ? ' width="64"' : ''}>
        </td>
    </tr>
`;
  1. Apply the same security measures as the web template:
-<img src="${dataset.imageUrl}" alt="CTA Image" class="kg-cta-image">
+<img src="${sanitizeUrl(dataset.imageUrl)}" alt="${dataset.imageAltText || 'CTA Image'}" class="kg-cta-image">

-<a href="${dataset.buttonUrl}" 
+<a href="${sanitizeUrl(dataset.buttonUrl)}"


return `
<div class="cta-card-email" style="${backgroundStyle} padding: 16px; text-align: center; border-radius: 8px;">
${dataset.hasImage ? `<img src="${dataset.imageUrl}" alt="CTA Image" style="max-width: 100%; border-radius: 4px;">` : ''}
<div class="cta-text" style="margin-top: 12px; color: ${dataset.textColor};">
${dataset.textValue}
</div>
${dataset.showButton ? `
<a href="${dataset.buttonUrl}" class="cta-button"
style="display: inline-block; margin-top: 12px; padding: 10px 16px;
${buttonStyle} color: ${dataset.buttonTextColor}; text-decoration: none;
border-radius: 4px;">
${dataset.buttonText}
</a>
` : ''}
<table class="kg-card kg-cta-card kg-cta-bg-${dataset.backgroundColor} kg-cta-${dataset.layout}" border="0" cellpadding="0" cellspacing="0" width="100%" style="${backgroundStyle}">
${dataset.hasSponsorLabel ? `
<div class="sponsor-label" style="margin-top: 8px; font-size: 12px; color: #888;">
Sponsored
</div>
<tr>
<td class="kg-cta-sponsor-label">
<p>Sponsored</p>
</td>
</tr>
` : ''}
</div>
${renderContent()}
</table>
`;
}

Expand All @@ -75,6 +158,11 @@ export function renderCallToActionNode(node, options = {}) {
textColor: node.textColor
};

// Add validation for backgroundColor
if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+$/)) {
dataset.backgroundColor = 'white';
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Align backgroundColor validation with buttonColor validation.

The validation pattern for backgroundColor is more restrictive than buttonColor. Consider using the same pattern for consistency.

-if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+$/)) {
+if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+|#([a-fA-F\d]{3}|[a-fA-F\d]{6})$/)) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Add validation for backgroundColor
if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+$/)) {
dataset.backgroundColor = 'white';
}
// Add validation for backgroundColor
if (!dataset.backgroundColor || !dataset.backgroundColor.match(/^[a-zA-Z\d-]+|#([a-fA-F\d]{3}|[a-fA-F\d]{6})$/)) {
dataset.backgroundColor = 'white';
}


if (options.target === 'email') {
const emailDoc = options.createDocument();
const emailDiv = emailDoc.createElement('div');
Expand Down
Loading