Couple of updates / fixes for the Image Carousel (#1663)

* updating to max-height instead and some other fixes

* updating from auto to none, and some placement of the control fixes

* auto position controls

* switched to opening a modal if clicked then a new tab to be consistent with regular image

* prettier fixes

Author: kgeller

docs/syntax/images.md

@@ -131,7 +131,7 @@ The image carousel directive builds upon the image directive.
 ::::{carousel}
 
 :id: nested-carousel-example
-:fixed-height: small ## small, medium, auto (auto is default if fixed-height is not specified)
+:max-height: small ## small, medium, none (none is default if max-height is not specified)
 
 :::{image} images/apm.png
 :alt: First image description
@@ -148,7 +148,7 @@ The image carousel directive builds upon the image directive.
 ::::{carousel}
 
 :id: nested-carousel-example
-:fixed-height: small
+:max-height: small
 
 :::{image} images/apm.png
 :alt: First image description

src/Elastic.Documentation.Site/Assets/image-carousel.ts

@@ -18,6 +18,13 @@ class ImageCarousel {
         this.slides = Array.from(
             this.container.querySelectorAll('.carousel-slide')
         )
+
+        // Don't initialize if no slides
+        if (this.slides.length === 0) {
+            console.warn('No carousel slides found')
+            return
+        }
+
         this.indicators = Array.from(
             this.container.querySelectorAll('.carousel-indicator')
         )
@@ -26,6 +33,7 @@ class ImageCarousel {
 
         this.initializeSlides()
         this.setupEventListeners()
+        this.positionControls()
     }
 
     private initializeSlides(): void {
@@ -60,6 +68,23 @@ class ImageCarousel {
             indicator.addEventListener('click', () => this.goToSlide(index))
         })
 
+        // Handle image clicks for modal
+        this.slides.forEach((slide) => {
+            const imageLink = slide.querySelector('.carousel-image-reference')
+            if (imageLink) {
+                imageLink.addEventListener('click', (e) => {
+                    e.preventDefault()
+                    const modalId = imageLink.getAttribute('data-modal-id')
+                    if (modalId) {
+                        const modal = document.getElementById(modalId)
+                        if (modal) {
+                            modal.style.display = 'flex'
+                        }
+                    }
+                })
+            }
+        })
+
         // Keyboard navigation
         document.addEventListener('keydown', (e) => {
             if (!this.isInViewport()) return
@@ -125,6 +150,69 @@ class ImageCarousel {
                 (window.innerWidth || document.documentElement.clientWidth)
         )
     }
+
+    private positionControls(): void {
+        if (!this.prevButton || !this.nextButton) return
+
+        // Wait for images to load before positioning
+        const images = Array.from(this.container.querySelectorAll('img'))
+        if (images.length === 0) return
+
+        let loadedCount = 0
+        const totalImages = images.length
+
+        const positionAfterLoad = () => {
+            loadedCount++
+            if (loadedCount === totalImages) {
+                this.calculateControlPosition()
+            }
+        }
+
+        images.forEach((img) => {
+            if (img.complete) {
+                positionAfterLoad()
+            } else {
+                img.addEventListener('load', positionAfterLoad)
+                img.addEventListener('error', positionAfterLoad) // Handle failed loads
+            }
+        })
+    }
+
+    private calculateControlPosition(): void {
+        if (!this.prevButton || !this.nextButton) return
+
+        const images = Array.from(this.container.querySelectorAll('img'))
+        let minHeight = Infinity
+
+        // Find the smallest image height among all images
+        images.forEach((img) => {
+            const height = img.offsetHeight
+            if (height > 0 && height < minHeight) {
+                minHeight = height
+            }
+        })
+
+        // Position controls at 40% the height of the smallest image
+        // But ensure a minimum distance from the top (50px) and don't go below 80% of the smallest image
+        if (minHeight !== Infinity && minHeight > 0) {
+            const fortyPercentHeight = Math.floor(minHeight * 0.4)
+            const minTop = 50 // Minimum 50px from top
+            const maxTop = Math.floor(minHeight * 0.8) // Maximum 80% down the smallest image
+
+            const controlTop = Math.max(
+                minTop,
+                Math.min(fortyPercentHeight, maxTop)
+            )
+
+            this.prevButton.style.top = `${controlTop}px`
+            this.nextButton.style.top = `${controlTop}px`
+
+            // Debug logging (remove in production)
+            console.log(
+                `Carousel controls positioned: minHeight=${minHeight}px, controlTop=${controlTop}px`
+            )
+        }
+    }
 }
 
 // Export function to initialize carousels
@@ -136,19 +224,22 @@ export function initImageCarousel(): void {
     carousels.forEach((carouselElement) => {
         const carousel = carouselElement as HTMLElement
 
-        // Get the existing track
+        // Skip if carousel already has slides (server-rendered)
+        const existingSlides = carousel.querySelectorAll('.carousel-slide')
+        if (existingSlides.length > 0) {
+            // Just initialize the existing carousel
+            new ImageCarousel(carousel)
+            return
+        }
+
+        // Get the existing track for dynamic carousels
         let track = carousel.querySelector('.carousel-track')
         if (!track) {
             track = document.createElement('div')
             track.className = 'carousel-track'
             carousel.appendChild(track)
         }
 
-        // Clean up any existing slides - this prevents duplicates
-        const existingSlides = Array.from(
-            track.querySelectorAll('.carousel-slide')
-        )
-
         // Find all image links that might be related to this carousel
         const section = findSectionForCarousel(carousel)
         if (!section) return

src/Elastic.Documentation.Site/Assets/markdown/image-carousel.css

@@ -2,13 +2,11 @@
 	position: relative;
 	width: 100%;
 	margin: 2rem 0;
-	overflow: hidden;
 }
 
 .carousel-track {
 	width: 100%;
 	position: relative;
-	min-height: 200px;
 }
 
 .carousel-slide {
@@ -27,19 +25,25 @@
 }
 
 .carousel-image-reference {
-	display: block;
+	display: flex;
+	justify-content: center;
+	align-items: center;
+	text-decoration: none;
+}
+
+.carousel-image-reference::after {
+	display: none !important;
 }
 
 .carousel-image-reference img {
-	width: 100%;
+	max-width: 100%;
 	height: auto;
 	display: block;
+	margin: 0 auto;
 }
 
 .carousel-control {
 	position: absolute;
-	top: 50%;
-	transform: translateY(-50%);
 	background-color: rgba(0, 0, 0, 0.5);
 	border: none;
 	color: white;
@@ -60,11 +64,11 @@
 }
 
 .carousel-prev {
-	left: 10px;
+	left: 0;
 }
 
 .carousel-next {
-	right: 10px;
+	right: 0;
 }
 
 .carousel-indicators {
@@ -92,38 +96,61 @@
 	background-color: black;
 }
 
-/* Fixed height carousel styles */
-.carousel-container[data-fixed-height] .carousel-track {
-	min-height: auto;
+/* Max height carousel styles */
+.carousel-container[data-max-height] {
+	padding-bottom: 40px; /* Space for indicators */
+}
+
+.carousel-container[data-max-height] .carousel-track {
+	max-height: var(--carousel-max-height);
 	overflow: hidden;
 }
 
-.carousel-container[data-fixed-height] .carousel-slide {
-	height: 100%;
-	top: 0;
-	left: 0;
+.carousel-container[data-max-height] .carousel-image-reference img {
+	max-height: var(--carousel-max-height);
+	width: auto;
 }
 
-.carousel-container[data-fixed-height] .carousel-slide[data-active='true'] {
-	position: relative;
-	height: 100%;
-	top: auto;
-	left: auto;
+/* None height carousel styles - images at natural size */
+.carousel-container[data-none-height] {
+	padding-bottom: 40px; /* Space for indicators */
 }
 
-.carousel-container[data-fixed-height] .carousel-image-reference {
-	height: 100%;
+/* Override modal styles for image carousels */
+.modal .modal-content {
+	max-width: 95vw !important;
+	max-height: 95vh !important;
+	width: auto !important;
+	height: auto !important;
+	padding: 0 !important;
+	background: transparent !important;
+	box-shadow: none !important;
+}
+
+.modal .modal-content img {
+	max-width: 95vw;
+	max-height: 95vh;
+	width: auto;
+	height: auto;
+	object-fit: contain;
+	display: block;
+}
+
+/* Ensure the close button is visible */
+.modal .modal-close {
+	background: rgba(0, 0, 0, 0.7);
+	border-radius: 50%;
+	width: 40px;
+	height: 40px;
 	display: flex;
 	align-items: center;
 	justify-content: center;
+	top: -20px;
+	right: -20px;
 }
 
-.carousel-container[data-fixed-height] .carousel-image-reference img {
-	width: auto;
-	height: 100%;
-	max-width: 100%;
-	object-fit: contain;
-	object-position: center;
+.modal .modal-close a {
+	color: white;
 }
 @media (max-width: 768px) {
 	.carousel-control {

src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs

@@ -138,7 +138,7 @@ private static void WriteImageCarousel(HtmlRenderer renderer, ImageCarouselBlock
 				Target = img.Target,
 				ImageUrl = img.ImageUrl
 			}).ToList(),
-			FixedHeight = block.FixedHeight
+			MaxHeight = block.MaxHeight
 		});
 		RenderRazorSlice(slice, renderer);
 	}

src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselBlock.cs

@@ -14,23 +14,23 @@ public class ImageCarouselBlock(DirectiveBlockParser parser, ParserContext conte
 {
 	public List<ImageBlock> Images { get; } = [];
 	public string? Id { get; set; }
-	public string? FixedHeight { get; set; }
+	public string? MaxHeight { get; set; }
 
 	public override string Directive => "carousel";
 
 	public override void FinalizeAndValidate(ParserContext context)
 	{
 		// Parse options
 		Id = Prop("id");
-		FixedHeight = Prop("fixed-height");
+		MaxHeight = Prop("max-height");
 
-		// Validate fixed-height option
-		if (!string.IsNullOrEmpty(FixedHeight))
+		// Validate max-height option
+		if (!string.IsNullOrEmpty(MaxHeight))
 		{
-			var validHeights = new[] { "auto", "small", "medium" };
-			if (!validHeights.Contains(FixedHeight.ToLower()))
+			var validHeights = new[] { "none", "small", "medium" };
+			if (!validHeights.Contains(MaxHeight.ToLower()))
 			{
-				this.EmitWarning($"Invalid fixed-height value '{FixedHeight}'. Valid options are: auto, small, medium. Defaulting to 'auto'.");
+				this.EmitWarning($"Invalid max-height value '{MaxHeight}'. Valid options are: none, small, medium. Defaulting to 'none'.");
 			}
 		}