From ac238493d09a5e36e17e0bae2abb8e95da01ccf5 Mon Sep 17 00:00:00 2001 From: Kylie Meli Date: Wed, 30 Jul 2025 16:47:27 -0400 Subject: [PATCH 1/5] updating to max-height instead and some other fixes --- .../Assets/image-carousel.ts | 7 +++ .../Assets/markdown/image-carousel.css | 55 +++++++++---------- .../Myst/Directives/DirectiveHtmlRenderer.cs | 2 +- .../Directives/Image/ImageCarouselBlock.cs | 12 ++-- .../Directives/Image/ImageCarouselView.cshtml | 17 +++--- .../Image/ImageCarouselViewModel.cs | 2 +- .../Directives/ImageCarouselTests.cs | 26 ++++----- 7 files changed, 64 insertions(+), 57 deletions(-) diff --git a/src/Elastic.Documentation.Site/Assets/image-carousel.ts b/src/Elastic.Documentation.Site/Assets/image-carousel.ts index 3f3a4b163..bcaa6333c 100644 --- a/src/Elastic.Documentation.Site/Assets/image-carousel.ts +++ b/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') ) diff --git a/src/Elastic.Documentation.Site/Assets/markdown/image-carousel.css b/src/Elastic.Documentation.Site/Assets/markdown/image-carousel.css index 1a5480e42..7a2deada5 100644 --- a/src/Elastic.Documentation.Site/Assets/markdown/image-carousel.css +++ b/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,22 @@ } .carousel-image-reference { - display: block; + display: flex; + justify-content: center; + align-items: center; } .carousel-image-reference img { - width: 100%; + max-width: 100%; height: auto; display: block; + box-sizing: border-box; + margin: 0 auto; } .carousel-control { position: absolute; - top: 50%; - transform: translateY(-50%); + top: 100px; /* Fixed distance from top */ background-color: rgba(0, 0, 0, 0.5); border: none; color: white; @@ -92,38 +93,34 @@ background-color: black; } -/* Fixed height carousel styles */ -.carousel-container[data-fixed-height] .carousel-track { - min-height: auto; - overflow: hidden; +/* Max height carousel styles */ +.carousel-container[data-max-height] { + padding-bottom: 40px; /* Space for indicators */ } -.carousel-container[data-fixed-height] .carousel-slide { - height: 100%; - top: 0; - left: 0; +.carousel-container[data-max-height] .carousel-track { + max-height: var(--carousel-max-height); + overflow: hidden; } -.carousel-container[data-fixed-height] .carousel-slide[data-active='true'] { - position: relative; - height: 100%; - top: auto; - left: auto; +.carousel-container[data-max-height] .carousel-image-reference img { + max-height: var(--carousel-max-height); + width: auto; + height: auto; + display: block; + margin: 0 auto; } -.carousel-container[data-fixed-height] .carousel-image-reference { - height: 100%; - display: flex; - align-items: center; - justify-content: center; +/* Auto height carousel styles - images at natural size */ +.carousel-container[data-auto-height] { + padding-bottom: 40px; /* Space for indicators */ } -.carousel-container[data-fixed-height] .carousel-image-reference img { - width: auto; - height: 100%; +.carousel-container[data-auto-height] .carousel-image-reference img { max-width: 100%; - object-fit: contain; - object-position: center; + height: auto; + display: block; + margin: 0 auto; } @media (max-width: 768px) { .carousel-control { diff --git a/src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs b/src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs index 4c8a6e2d6..e11b5a91a 100644 --- a/src/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cs +++ b/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); } diff --git a/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselBlock.cs b/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselBlock.cs index c5597c4ff..60e3fc5a4 100644 --- a/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselBlock.cs +++ b/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselBlock.cs @@ -14,7 +14,7 @@ public class ImageCarouselBlock(DirectiveBlockParser parser, ParserContext conte { public List Images { get; } = []; public string? Id { get; set; } - public string? FixedHeight { get; set; } + public string? MaxHeight { get; set; } public override string Directive => "carousel"; @@ -22,15 +22,15 @@ 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())) + 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: auto, small, medium. Defaulting to 'auto'."); } } diff --git a/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselView.cshtml b/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselView.cshtml index 1d59c6d78..f1a457c9d 100644 --- a/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselView.cshtml +++ b/src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselView.cshtml @@ -1,25 +1,28 @@ @inherits RazorSlice @{ // Convert semantic height values to pixel values - string? pixelHeight = Model.FixedHeight?.ToLower() switch + var maxHeightValue = Model.MaxHeight?.ToLower(); + string? pixelHeight = maxHeightValue switch { "small" => "350px", "medium" => "750px", - "auto" or null or "" => null, + "auto" => "none", // Auto means no max-height constraint + null or "" => null, _ => null // Default to none for invalid values }; - var hasFixedHeight = !string.IsNullOrEmpty(pixelHeight); - var trackStyle = hasFixedHeight ? $"height: {pixelHeight};" : ""; + var hasMaxHeight = !string.IsNullOrEmpty(pixelHeight) && maxHeightValue != null; + var containerStyle = hasMaxHeight ? $"--carousel-max-height: {pixelHeight};" : ""; + var dataAttribute = maxHeightValue == "auto" ? "data-auto-height" : (hasMaxHeight ? "data-max-height" : ""); } -