Improve template preview images in summary fields

[jsirish]

This PR enhances the preview image display in the Element Templates admin GridField.

Changes

Summary Fields Reordering

• Moved Preview Image to first column for better visibility
• Order now: Preview Image → Title → Page Type

Click-to-Enlarge Functionality

• Added custom getLayoutImageThumbnail() method
• Thumbnails scaled to 200px width maintaining aspect ratio
• Click thumbnail to view 800px enlarged version in dark overlay
• event.stopPropagation() prevents GridField row click navigation
• Images render as DBHTMLText for proper display (not escaped)

User Experience Improvements

• Preview images no longer cut off in GridField
• Dark overlay (rgba 0,0,0,0.8) with centered enlarged image
• Click anywhere on overlay to close
• Hover cursor indicates clickability

Testing

• Tested with 5 element templates (3-column cards, hero+CTA, FAQ, two-column, product detail)
• Click-to-enlarge working in all templates
• No navigation conflicts with GridField row clicks

Resolves issue where preview images were cut off making it difficult for editors to see full template layouts.

[Copilot]

Pull request overview

This PR enhances the Element Templates admin interface by improving preview image display and adding click-to-enlarge functionality for template layout images in the GridField.

Key Changes

• Reordered summary fields to display Preview Image in the first column for better visibility
• Added custom getLayoutImageThumbnail() method that generates 200px thumbnails with inline JavaScript for click-to-enlarge overlay functionality
• Overlay displays 800px enlarged version with dark background and click-to-close behavior

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The click-to-enlarge overlay implementation has several accessibility issues:

1. Keyboard Navigation: Users cannot open or close the overlay using keyboard (no Enter key handler, no Escape key to close)
2. Screen Reader Support: No ARIA labels or roles to indicate the image is clickable or that an overlay is present
3. Focus Management: When the overlay opens, focus is not trapped or moved to the overlay, and when it closes, focus is not restored
4. Alt Text: While alt text is included, there's no indication for screen reader users that the image can be clicked for a larger version

Consider adding:

• role="button" and aria-label attributes
• Keyboard event handlers (Enter/Space to open, Escape to close)
• Focus trap within the overlay
• ARIA live region announcements when overlay opens/closes

Suggested change

	<img src="%s" alt="%s" style="max-width: 200px; height: auto; display: block; cursor: pointer;"
	onclick="
	event.stopPropagation();
	var overlay = document.createElement(\'div\');
	overlay.style.cssText = \'position: fixed; top: 0; left: 0; width: 100%%; height: 100%%; background: rgba(0,0,0,0.8); z-index: 10000; display: flex; align-items: center; justify-content: center; cursor: pointer;\';
	var img = document.createElement(\'img\');
	img.src = \'%s\';
	img.alt = \'%s\';
	img.style.cssText = \'max-width: 90%%; max-height: 90%%; box-shadow: 0 0 20px rgba(0,0,0,0.5);\';
	overlay.appendChild(img);
	overlay.onclick = function() { document.body.removeChild(overlay); };
	document.body.appendChild(overlay);
	"
	title="Click to view larger" />
	</div>',
	$thumbnailUrl,
	$title,
	<img src="%s"
	alt="%s"
	style="max-width: 200px; height: auto; display: block; cursor: pointer;"
	role="button"
	tabindex="0"
	aria-label="View larger version of %s"
	onclick="
	event.stopPropagation();
	var previouslyFocused = document.activeElement;
	var overlay = document.createElement(\'div\');
	overlay.setAttribute(\'role\', \'dialog\');
	overlay.setAttribute(\'aria-modal\', \'true\');
	overlay.style.cssText = \'position: fixed; top: 0; left: 0; width: 100%%; height: 100%%; background: rgba(0,0,0,0.8); z-index: 10000; display: flex; align-items: center; justify-content: center; cursor: pointer;\';
	overlay.tabIndex = -1;
	var img = document.createElement(\'img\');
	img.src = \'%s\';
	img.alt = \'%s\';
	img.style.cssText = \'max-width: 90%%; max-height: 90%%; box-shadow: 0 0 20px rgba(0,0,0,0.5);\';
	overlay.appendChild(img);
	var closeOverlay = function() {
	if (overlay.parentNode) {
	overlay.parentNode.removeChild(overlay);
	if (previouslyFocused && typeof previouslyFocused.focus === \'function\') {
	previouslyFocused.focus();
	}
	}
	};
	overlay.onclick = function(event) {
	if (event.target === overlay) {
	closeOverlay();
	}
	};
	overlay.addEventListener(\'keydown\', function(e) {
	if (e.key === \'Escape\' || e.key === \'Esc\') {
	e.preventDefault();
	closeOverlay();
	} else if (e.key === \'Tab\') {
	e.preventDefault();
	overlay.focus();
	}
	});
	document.body.appendChild(overlay);
	overlay.focus();
	"
	onkeydown="
	if (event.key === \'Enter\' || event.key === \' \' || event.key === \'Spacebar\') {
	event.preventDefault();
	this.click();
	}
	"
	title="Click to view larger" />
	</div>',
	$thumbnailUrl,
	$title,
	$title,

[Copilot]

The overlay removal logic has a potential bug when multiple overlays are created in quick succession. If a user clicks multiple thumbnails rapidly before closing the first overlay, calling document.body.removeChild(overlay) will attempt to remove the specific overlay that was clicked, but clicking on the overlay itself will try to remove it from the DOM. If the overlay has already been removed or if multiple overlays exist, this could cause JavaScript errors.

Additionally, there's no cleanup for the dynamically created DOM elements if the GridField is refreshed or the page is navigated away from while an overlay is open, potentially causing memory leaks.

Consider adding:

• A check to ensure the overlay still exists before attempting removal
• A mechanism to prevent multiple overlays from being created simultaneously
• Proper cleanup handlers

Suggested change

	var overlay = document.createElement(\'div\');
	overlay.style.cssText = \'position: fixed; top: 0; left: 0; width: 100%%; height: 100%%; background: rgba(0,0,0,0.8); z-index: 10000; display: flex; align-items: center; justify-content: center; cursor: pointer;\';
	var img = document.createElement(\'img\');
	img.src = \'%s\';
	img.alt = \'%s\';
	img.style.cssText = \'max-width: 90%%; max-height: 90%%; box-shadow: 0 0 20px rgba(0,0,0,0.5);\';
	overlay.appendChild(img);
	overlay.onclick = function() { document.body.removeChild(overlay); };
	document.body.appendChild(overlay);
	(function() {
	// Remove any existing overlay to prevent multiples
	if (window.__templateOverlay && window.__templateOverlay.parentNode) {
	window.__templateOverlay.parentNode.removeChild(window.__templateOverlay);
	window.__templateOverlay = null;
	}
	var overlay = document.createElement(\'div\');
	overlay.style.cssText = \'position: fixed; top: 0; left: 0; width: 100%%; height: 100%%; background: rgba(0,0,0,0.8); z-index: 10000; display: flex; align-items: center; justify-content: center; cursor: pointer;\';
	var img = document.createElement(\'img\');
	img.src = \'%s\';
	img.alt = \'%s\';
	img.style.cssText = \'max-width: 90%%; max-height: 90%%; box-shadow: 0 0 20px rgba(0,0,0,0.5);\';
	overlay.appendChild(img);
	overlay.onclick = function() {
	if (overlay && overlay.parentNode) {
	overlay.parentNode.removeChild(overlay);
	if (window.__templateOverlay === overlay) {
	window.__templateOverlay = null;
	}
	}
	};
	document.body.appendChild(overlay);
	window.__templateOverlay = overlay;
	// Register a one-time cleanup handler for page unload
	if (!window.__templateOverlayCleanupRegistered) {
	window.addEventListener(\'beforeunload\', function() {
	if (window.__templateOverlay && window.__templateOverlay.parentNode) {
	window.__templateOverlay.parentNode.removeChild(window.__templateOverlay);
	window.__templateOverlay = null;
	}
	});
	window.__templateOverlayCleanupRegistered = true;
	}
	})();

[Copilot]

The new getLayoutImageThumbnail() method lacks test coverage. The existing TemplateTest.php file contains tests for other Template model methods (getCMSFields, canCreate, canEdit, canDelete, etc.), but does not test this new method.

Consider adding tests to verify:

• The method returns an empty string when no LayoutImage exists
• The method returns proper HTML with correct thumbnail and full image URLs when an image exists
• The HTML output contains the expected structure and attributes
• The method handles edge cases like missing or invalid images gracefully

[Copilot]

The image scaling operations ScaleWidth(200) and ScaleWidth(800) will be executed every time the GridField is rendered for each Template row. If there are many templates in the GridField, this could cause performance issues as:

1. Each image needs to be processed/resized on every page load
2. The image manipulation happens in PHP on each request rather than being cached

SilverStripe's image manipulation typically creates cached versions, but repeatedly calling these methods on every GridField render could still impact performance with a large number of templates.

Consider:

• Verifying that SilverStripe's image caching is functioning properly for these scaled versions
• Adding appropriate caching headers
• If performance becomes an issue, consider pre-generating and caching the HTML output

Suggested change

	// Get a scaled version that maintains aspect ratio
	$thumbnail = $this->LayoutImage()->ScaleWidth(200);
	$fullImage = $this->LayoutImage()->ScaleWidth(800);
	if ($thumbnail && $fullImage) {
	$thumbnailUrl = $thumbnail->getURL();
	$fullUrl = $fullImage->getURL();
	// Get a scaled version that maintains aspect ratio for the thumbnail
	$thumbnail = $this->LayoutImage()->ScaleWidth(200);
	// Use the original image URL for the enlarged view to avoid extra scaling
	$fullUrl = $this->LayoutImage()->getURL();
	if ($thumbnail) {
	$thumbnailUrl = $thumbnail->getURL();

[Copilot]

The method returns different types depending on the condition: DBHTMLText when an image exists (line 294) and an empty string when it doesn't (line 298). This inconsistency in return types can lead to unexpected behavior and makes the code harder to reason about.

For better type consistency and to match the return type hint, the method should always return DBHTMLText|string consistently, or ideally always return DBHTMLText with an empty value when there's no image.

Consider changing line 298 to return DBHTMLText::create()->setValue('') instead of just an empty string.

[Copilot]

The onclick handler builds JavaScript using %s placeholders that are filled with thumbnailUrl, fullUrl, and especially title, which comes from $this->Title and is only HTML-escaped via htmlspecialchars rather than escaped for JavaScript context. Because HTML entities like ' are decoded before the inline handler is parsed as JavaScript, a title containing a single quote and JS code (e.g. ');alert(1);//) will break out of the string literal in img.alt = '%s'; and inject arbitrary script, leading to stored XSS in the CMS GridField when an editor clicks the thumbnail. To fix this, ensure that any dynamic values interpolated into the inline JS (both title and the image URLs) are properly escaped for JavaScript string context or, preferably, avoid inline JS entirely by wiring the click behavior via unobtrusive JavaScript using data attributes or DOM event listeners.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The thumbnail URL should be HTML-escaped before being inserted into the src attribute. While getURL() typically returns safe paths, it's a security best practice to explicitly escape all dynamic content inserted into HTML attributes to prevent potential XSS vulnerabilities.

[Copilot]

This method contains a significant amount of inline JavaScript (lines 282-329) which makes it difficult to test, maintain, and could impact performance when rendering many rows. Consider extracting this JavaScript to a separate file and including it as a resource with Requirements::javascript(), or implementing this as a React component if using the Silverstripe admin UI framework. The JavaScript could register a global click handler that operates on data attributes attached to the thumbnail images.