-
-
Notifications
You must be signed in to change notification settings - Fork 2
Improve template preview images in summary fields #36
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| use SilverStripe\Forms\DropdownField; | ||
| use SilverStripe\Forms\FieldList; | ||
| use SilverStripe\ORM\DataObject; | ||
| use SilverStripe\ORM\FieldType\DBHTMLText; | ||
| use SilverStripe\Security\Member; | ||
| use SilverStripe\Security\PermissionProvider; | ||
| use SilverStripe\Security\Security; | ||
|
|
@@ -106,8 +107,8 @@ class Template extends DataObject implements PermissionProvider | |
| * @config | ||
| */ | ||
| private static array $summary_fields = [ | ||
| 'LayoutImageThumbnail' => 'Preview Image', | ||
| 'Title' => 'Layout Name', | ||
| 'LayoutImage.CMSThumbnail' => 'Preview Image', | ||
| 'PageTypeName' => 'Page Type', | ||
| ]; | ||
|
|
||
|
|
@@ -249,6 +250,105 @@ public function PageTypeName(): string | |
| return singleton($this->PageType)->singular_name(); | ||
| } | ||
|
|
||
| /** | ||
| * Returns a better sized thumbnail for the layout image in summary fields. | ||
| * Includes click-to-enlarge functionality with accessibility support. | ||
| * | ||
| * @return DBHTMLText | ||
| */ | ||
| public function getLayoutImageThumbnail() | ||
| { | ||
| if ($this->LayoutImage() && $this->LayoutImage()->exists()) { | ||
| // Get a scaled version that maintains aspect ratio | ||
| $thumbnail = $this->LayoutImage()->ScaleWidth(200); | ||
| // Use original image for enlarged view to avoid extra scaling | ||
| $fullUrl = $this->LayoutImage()->getURL(); | ||
|
|
||
| if ($thumbnail) { | ||
| $thumbnailUrl = $thumbnail->getURL(); | ||
| // Properly escape for JavaScript context | ||
| $title = htmlspecialchars($this->Title, ENT_QUOTES, 'UTF-8'); | ||
| $titleJs = json_encode($this->Title); | ||
| $fullUrlJs = json_encode($fullUrl); | ||
|
|
||
| $html = sprintf( | ||
| '<div style="position: relative; display: inline-block;"> | ||
| <img src="%s" | ||
| alt="%s" | ||
| role="button" | ||
| tabindex="0" | ||
| aria-label="View larger version of %s" | ||
| style="max-width: 200px; height: auto; display: block; cursor: pointer;" | ||
| onclick=" | ||
| event.stopPropagation(); | ||
| // Remove any existing overlay | ||
| if (window.__templateOverlay && window.__templateOverlay.parentNode) { | ||
| window.__templateOverlay.parentNode.removeChild(window.__templateOverlay); | ||
| window.__templateOverlay = null; | ||
| } | ||
| var previouslyFocused = document.activeElement; | ||
| var overlay = document.createElement(\'div\'); | ||
| overlay.setAttribute(\'role\', \'dialog\'); | ||
| overlay.setAttribute(\'aria-modal\', \'true\'); | ||
| overlay.setAttribute(\'aria-label\', \'Enlarged template preview\'); | ||
| 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 && overlay.parentNode) { | ||
| overlay.parentNode.removeChild(overlay); | ||
| if (window.__templateOverlay === overlay) { | ||
| window.__templateOverlay = null; | ||
| } | ||
| 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); | ||
| window.__templateOverlay = overlay; | ||
| overlay.focus(); | ||
| " | ||
|
Comment on lines
+282
to
+329
|
||
| onkeydown=" | ||
| if (event.key === \'Enter\' || event.key === \' \' || event.key === \'Spacebar\') { | ||
| event.preventDefault(); | ||
| this.click(); | ||
| } | ||
| " | ||
| title="Click to view larger" /> | ||
| </div>', | ||
| $thumbnailUrl, | ||
| $title, | ||
| $title, | ||
| $fullUrlJs, | ||
| $titleJs | ||
| ); | ||
|
|
||
| return DBHTMLText::create()->setValue($html); | ||
| } | ||
| } | ||
|
|
||
| return DBHTMLText::create()->setValue(''); | ||
| } | ||
|
|
||
| /** | ||
| * @return string | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| <?php | ||
|
|
||
| namespace Dynamic\ElememtalTemplates\Tests\Models; | ||
|
|
||
| use Dynamic\ElememtalTemplates\Models\Template; | ||
| use SilverStripe\Assets\Image; | ||
| use SilverStripe\Dev\SapphireTest; | ||
| use SilverStripe\ORM\FieldType\DBHTMLText; | ||
|
|
||
| /** | ||
| * Tests for Template layout image thumbnail functionality | ||
| */ | ||
| class TemplateLayoutImageTest extends SapphireTest | ||
| { | ||
| protected static $fixture_file = '../fixtures.yml'; | ||
|
|
||
| /** | ||
| * Test that getLayoutImageThumbnail returns empty DBHTMLText when no image exists | ||
| */ | ||
| public function testGetLayoutImageThumbnailWithNoImage() | ||
| { | ||
| $template = Template::create(); | ||
| $template->Title = 'Test Template'; | ||
| $template->write(); | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
|
|
||
| $this->assertInstanceOf(DBHTMLText::class, $result); | ||
| $this->assertEquals('', $result->getValue()); | ||
| } | ||
|
|
||
| /** | ||
| * Test that getLayoutImageThumbnail returns proper HTML structure | ||
| */ | ||
| public function testGetLayoutImageThumbnailWithImage() | ||
| { | ||
| $template = $this->objFromFixture(Template::class, 'template1'); | ||
|
|
||
| if (!$template->LayoutImage() || !$template->LayoutImage()->exists()) { | ||
| $this->markTestSkipped('Fixture template1 does not have a layout image'); | ||
| } | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
|
|
||
| $this->assertInstanceOf(DBHTMLText::class, $result); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check for expected HTML structure | ||
| $this->assertStringContainsString('<img', $html); | ||
| $this->assertStringContainsString('role="button"', $html); | ||
| $this->assertStringContainsString('tabindex="0"', $html); | ||
| $this->assertStringContainsString('aria-label=', $html); | ||
| $this->assertStringContainsString('max-width: 200px', $html); | ||
| $this->assertStringContainsString('onclick=', $html); | ||
| $this->assertStringContainsString('onkeydown=', $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that title is properly escaped in HTML output | ||
| */ | ||
| public function testGetLayoutImageThumbnailEscapesTitle() | ||
| { | ||
| $template = Template::create(); | ||
| $template->Title = 'Test <script>alert("xss")</script> Template'; | ||
| $template->write(); | ||
|
|
||
| // Create a mock image | ||
| $image = Image::create(); | ||
| $image->Filename = 'test.jpg'; | ||
| $image->write(); | ||
|
|
||
| $template->LayoutImageID = $image->ID; | ||
| $template->write(); | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check that script tags are escaped in alt attribute | ||
| $this->assertStringNotContainsString('<script>', $html); | ||
| $this->assertStringContainsString('<script>', $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that JavaScript values are properly JSON-encoded | ||
| */ | ||
| public function testGetLayoutImageThumbnailJSONEncodesValues() | ||
| { | ||
| $template = Template::create(); | ||
| $template->Title = "Test's \"Template\" with 'quotes'"; | ||
| $template->write(); | ||
|
|
||
| // Create a mock image | ||
| $image = Image::create(); | ||
| $image->Filename = 'test.jpg'; | ||
| $image->write(); | ||
|
|
||
| $template->LayoutImageID = $image->ID; | ||
| $template->write(); | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check that values are JSON-encoded in JavaScript context | ||
| // Single quotes and double quotes should be properly escaped | ||
| $this->assertStringContainsString('img.alt = ', $html); | ||
| // Should not contain unescaped quotes that could break JS | ||
| $this->assertStringNotContainsString("img.alt = 'Test's", $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that accessibility attributes are present | ||
| */ | ||
| public function testGetLayoutImageThumbnailHasAccessibilityAttributes() | ||
| { | ||
| $template = $this->objFromFixture(Template::class, 'template1'); | ||
|
|
||
| if (!$template->LayoutImage() || !$template->LayoutImage()->exists()) { | ||
| $this->markTestSkipped('Fixture template1 does not have a layout image'); | ||
| } | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check for accessibility attributes | ||
| $this->assertMatchesRegularExpression('/role=["\']button["\']/', $html); | ||
| $this->assertMatchesRegularExpression('/tabindex=["\']0["\']/', $html); | ||
| $this->assertMatchesRegularExpression('/aria-label=/', $html); | ||
|
|
||
| // Check for keyboard event handler | ||
| $this->assertStringContainsString('onkeydown=', $html); | ||
| $this->assertStringContainsString('Enter', $html); | ||
| $this->assertStringContainsString('Spacebar', $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that overlay has proper ARIA attributes | ||
| */ | ||
| public function testGetLayoutImageThumbnailOverlayHasARIA() | ||
| { | ||
| $template = $this->objFromFixture(Template::class, 'template1'); | ||
|
|
||
| if (!$template->LayoutImage() || !$template->LayoutImage()->exists()) { | ||
| $this->markTestSkipped('Fixture template1 does not have a layout image'); | ||
| } | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check that overlay creation includes ARIA attributes | ||
| $this->assertStringContainsString("setAttribute('role', 'dialog')", $html); | ||
| $this->assertStringContainsString("setAttribute('aria-modal', 'true')", $html); | ||
| $this->assertStringContainsString("setAttribute('aria-label', 'Enlarged template preview')", $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that the method prevents multiple overlays | ||
| */ | ||
| public function testGetLayoutImageThumbnailPreventsMultipleOverlays() | ||
| { | ||
| $template = $this->objFromFixture(Template::class, 'template1'); | ||
|
|
||
| if (!$template->LayoutImage() || !$template->LayoutImage()->exists()) { | ||
| $this->markTestSkipped('Fixture template1 does not have a layout image'); | ||
| } | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| // Check that the JavaScript removes existing overlays | ||
| $this->assertStringContainsString('window.__templateOverlay', $html); | ||
| $this->assertStringContainsString('removeChild(window.__templateOverlay)', $html); | ||
| } | ||
|
|
||
| /** | ||
| * Test that event.stopPropagation is called to prevent row navigation | ||
| */ | ||
| public function testGetLayoutImageThumbnailStopsPropagation() | ||
| { | ||
| $template = $this->objFromFixture(Template::class, 'template1'); | ||
|
|
||
| if (!$template->LayoutImage() || !$template->LayoutImage()->exists()) { | ||
| $this->markTestSkipped('Fixture template1 does not have a layout image'); | ||
| } | ||
|
|
||
| $result = $template->getLayoutImageThumbnail(); | ||
| $html = $result->getValue(); | ||
|
|
||
| $this->assertStringContainsString('event.stopPropagation()', $html); | ||
| } | ||
| } |
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.
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.