Skip to content

Commit ac23849

Browse files
kgellerkgeller
committed
updating to max-height instead and some other fixes
1 parent ad418ac commit ac23849

File tree

7 files changed

+64
-57
lines changed

7 files changed

+64
-57
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ class ImageCarousel {
1818
this.slides = Array.from(
1919
this.container.querySelectorAll('.carousel-slide')
2020
)
21+
22+
// Don't initialize if no slides
23+
if (this.slides.length === 0) {
24+
console.warn('No carousel slides found')
25+
return
26+
}
27+
2128
this.indicators = Array.from(
2229
this.container.querySelectorAll('.carousel-indicator')
2330
)

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

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@
22
position: relative;
33
width: 100%;
44
margin: 2rem 0;
5-
overflow: hidden;
65
}
76

87
.carousel-track {
98
width: 100%;
109
position: relative;
11-
min-height: 200px;
1210
}
1311

1412
.carousel-slide {
@@ -27,19 +25,22 @@
2725
}
2826

2927
.carousel-image-reference {
30-
display: block;
28+
display: flex;
29+
justify-content: center;
30+
align-items: center;
3131
}
3232

3333
.carousel-image-reference img {
34-
width: 100%;
34+
max-width: 100%;
3535
height: auto;
3636
display: block;
37+
box-sizing: border-box;
38+
margin: 0 auto;
3739
}
3840

3941
.carousel-control {
4042
position: absolute;
41-
top: 50%;
42-
transform: translateY(-50%);
43+
top: 100px; /* Fixed distance from top */
4344
background-color: rgba(0, 0, 0, 0.5);
4445
border: none;
4546
color: white;
@@ -92,38 +93,34 @@
9293
background-color: black;
9394
}
9495

95-
/* Fixed height carousel styles */
96-
.carousel-container[data-fixed-height] .carousel-track {
97-
min-height: auto;
98-
overflow: hidden;
96+
/* Max height carousel styles */
97+
.carousel-container[data-max-height] {
98+
padding-bottom: 40px; /* Space for indicators */
9999
}
100100

101-
.carousel-container[data-fixed-height] .carousel-slide {
102-
height: 100%;
103-
top: 0;
104-
left: 0;
101+
.carousel-container[data-max-height] .carousel-track {
102+
max-height: var(--carousel-max-height);
103+
overflow: hidden;
105104
}
106105

107-
.carousel-container[data-fixed-height] .carousel-slide[data-active='true'] {
108-
position: relative;
109-
height: 100%;
110-
top: auto;
111-
left: auto;
106+
.carousel-container[data-max-height] .carousel-image-reference img {
107+
max-height: var(--carousel-max-height);
108+
width: auto;
109+
height: auto;
110+
display: block;
111+
margin: 0 auto;
112112
}
113113

114-
.carousel-container[data-fixed-height] .carousel-image-reference {
115-
height: 100%;
116-
display: flex;
117-
align-items: center;
118-
justify-content: center;
114+
/* Auto height carousel styles - images at natural size */
115+
.carousel-container[data-auto-height] {
116+
padding-bottom: 40px; /* Space for indicators */
119117
}
120118

121-
.carousel-container[data-fixed-height] .carousel-image-reference img {
122-
width: auto;
123-
height: 100%;
119+
.carousel-container[data-auto-height] .carousel-image-reference img {
124120
max-width: 100%;
125-
object-fit: contain;
126-
object-position: center;
121+
height: auto;
122+
display: block;
123+
margin: 0 auto;
127124
}
128125
@media (max-width: 768px) {
129126
.carousel-control {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ private static void WriteImageCarousel(HtmlRenderer renderer, ImageCarouselBlock
138138
Target = img.Target,
139139
ImageUrl = img.ImageUrl
140140
}).ToList(),
141-
FixedHeight = block.FixedHeight
141+
MaxHeight = block.MaxHeight
142142
});
143143
RenderRazorSlice(slice, renderer);
144144
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,23 @@ public class ImageCarouselBlock(DirectiveBlockParser parser, ParserContext conte
1414
{
1515
public List<ImageBlock> Images { get; } = [];
1616
public string? Id { get; set; }
17-
public string? FixedHeight { get; set; }
17+
public string? MaxHeight { get; set; }
1818

1919
public override string Directive => "carousel";
2020

2121
public override void FinalizeAndValidate(ParserContext context)
2222
{
2323
// Parse options
2424
Id = Prop("id");
25-
FixedHeight = Prop("fixed-height");
25+
MaxHeight = Prop("max-height");
2626

27-
// Validate fixed-height option
28-
if (!string.IsNullOrEmpty(FixedHeight))
27+
// Validate max-height option
28+
if (!string.IsNullOrEmpty(MaxHeight))
2929
{
3030
var validHeights = new[] { "auto", "small", "medium" };
31-
if (!validHeights.Contains(FixedHeight.ToLower()))
31+
if (!validHeights.Contains(MaxHeight.ToLower()))
3232
{
33-
this.EmitWarning($"Invalid fixed-height value '{FixedHeight}'. Valid options are: auto, small, medium. Defaulting to 'auto'.");
33+
this.EmitWarning($"Invalid max-height value '{MaxHeight}'. Valid options are: auto, small, medium. Defaulting to 'auto'.");
3434
}
3535
}
3636

‎src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselView.cshtml‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,28 @@
11
@inherits RazorSlice<Elastic.Markdown.Myst.Directives.Image.ImageCarouselViewModel>
22
@{
33
// Convert semantic height values to pixel values
4-
string? pixelHeight = Model.FixedHeight?.ToLower() switch
4+
var maxHeightValue = Model.MaxHeight?.ToLower();
5+
string? pixelHeight = maxHeightValue switch
56
{
67
"small" => "350px",
78
"medium" => "750px",
8-
"auto" or null or "" => null,
9+
"auto" => "none", // Auto means no max-height constraint
10+
null or "" => null,
911
_ => null // Default to none for invalid values
1012
};
1113

12-
var hasFixedHeight = !string.IsNullOrEmpty(pixelHeight);
13-
var trackStyle = hasFixedHeight ? $"height: {pixelHeight};" : "";
14+
var hasMaxHeight = !string.IsNullOrEmpty(pixelHeight) && maxHeightValue != null;
15+
var containerStyle = hasMaxHeight ? $"--carousel-max-height: {pixelHeight};" : "";
16+
var dataAttribute = maxHeightValue == "auto" ? "data-auto-height" : (hasMaxHeight ? "data-max-height" : "");
1417
}
15-
<div class="carousel-container" @(hasFixedHeight ? "data-fixed-height" : "")>
16-
<div class="carousel-track" style="@trackStyle">
18+
<div class="carousel-container" @dataAttribute style="@containerStyle">
19+
<div class="carousel-track">
1720
@for (int i = 0; i < Model.Images.Count; i++)
1821
{
1922
var image = Model.Images[i];
2023
<div class="carousel-slide" data-index="@i" @(i == 0 ? "data-active=true" : "")>
2124
<a class="carousel-image-reference" href="@image.ImageUrl" target="_blank">
22-
<img loading="lazy" title="@image.Title" alt="@(image.Alt ?? "")" src="@image.ImageUrl" style="@image.Style" />
25+
<img title="@image.Title" alt="@(image.Alt ?? "")" src="@image.ImageUrl" style="@image.Style" />
2326
</a>
2427
</div>
2528
}

‎src/Elastic.Markdown/Myst/Directives/Image/ImageCarouselViewModel.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ namespace Elastic.Markdown.Myst.Directives.Image;
1111
public class ImageCarouselViewModel : DirectiveViewModel
1212
{
1313
public required List<ImageViewModel> Images { get; init; }
14-
public string? FixedHeight { get; init; }
14+
public string? MaxHeight { get; init; }
1515
}

‎tests/Elastic.Markdown.Tests/Directives/ImageCarouselTests.cs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Elastic.Markdown.Tests.Directives;
1313
public class ImageCarouselBlockTests(ITestOutputHelper output) : DirectiveTest<ImageCarouselBlock>(output,
1414
"""
1515
:::{carousel}
16-
:fixed-height: medium
16+
:max-height: medium
1717
1818
```{image} img/image1.png
1919
:alt: First image
@@ -38,7 +38,7 @@ protected override void AddToFileSystem(MockFileSystem fileSystem)
3838
[Fact]
3939
public void ParsesCarouselProperties()
4040
{
41-
Block!.FixedHeight.Should().Be("medium");
41+
Block!.MaxHeight.Should().Be("medium");
4242
}
4343

4444
[Fact]
@@ -62,7 +62,7 @@ public void AllImagesFoundSoNoErrorIsEmitted()
6262
public class ImageCarouselWithSmallHeightTests(ITestOutputHelper output) : DirectiveTest<ImageCarouselBlock>(output,
6363
"""
6464
:::{carousel}
65-
:fixed-height: small
65+
:max-height: small
6666
6767
```{image} img/small.png
6868
:alt: Small image
@@ -75,17 +75,17 @@ protected override void AddToFileSystem(MockFileSystem fileSystem) =>
7575
fileSystem.AddFile(@"docs/img/small.png", "");
7676

7777
[Fact]
78-
public void ParsesSmallFixedHeight()
78+
public void ParsesSmallMaxHeight()
7979
{
80-
Block!.FixedHeight.Should().Be("small");
80+
Block!.MaxHeight.Should().Be("small");
8181
Collector.Diagnostics.Count.Should().Be(0);
8282
}
8383
}
8484

8585
public class ImageCarouselWithAutoHeightTests(ITestOutputHelper output) : DirectiveTest<ImageCarouselBlock>(output,
8686
"""
8787
:::{carousel}
88-
:fixed-height: auto
88+
:max-height: auto
8989
9090
```{image} img/auto.png
9191
:alt: Auto height image
@@ -98,17 +98,17 @@ protected override void AddToFileSystem(MockFileSystem fileSystem) =>
9898
fileSystem.AddFile(@"docs/img/auto.png", "");
9999

100100
[Fact]
101-
public void ParsesAutoFixedHeight()
101+
public void ParsesAutoMaxHeight()
102102
{
103-
Block!.FixedHeight.Should().Be("auto");
103+
Block!.MaxHeight.Should().Be("auto");
104104
Collector.Diagnostics.Count.Should().Be(0);
105105
}
106106
}
107107

108108
public class ImageCarouselWithInvalidHeightTests(ITestOutputHelper output) : DirectiveTest<ImageCarouselBlock>(output,
109109
"""
110110
:::{carousel}
111-
:fixed-height: large
111+
:max-height: large
112112
113113
```{image} img/invalid.png
114114
:alt: Invalid height image
@@ -121,15 +121,15 @@ protected override void AddToFileSystem(MockFileSystem fileSystem) =>
121121
fileSystem.AddFile(@"docs/img/invalid.png", "");
122122

123123
[Fact]
124-
public void WarnsOnInvalidFixedHeight()
124+
public void WarnsOnInvalidMaxHeight()
125125
{
126-
Block!.FixedHeight.Should().Be("large");
126+
Block!.MaxHeight.Should().Be("large");
127127

128128
Collector.Diagnostics.Should().HaveCount(1)
129129
.And.OnlyContain(d => d.Severity == Severity.Warning);
130130

131131
var warning = Collector.Diagnostics.First();
132-
warning.Message.Should().Contain("Invalid fixed-height value 'large'");
132+
warning.Message.Should().Contain("Invalid max-height value 'large'");
133133
warning.Message.Should().Contain("Valid options are: auto, small, medium");
134134
}
135135
}
@@ -171,7 +171,7 @@ protected override void AddToFileSystem(MockFileSystem fileSystem) =>
171171
[Fact]
172172
public void ParsesMinimalCarousel()
173173
{
174-
Block!.FixedHeight.Should().BeNull();
174+
Block!.MaxHeight.Should().BeNull();
175175
Block!.Images.Should().HaveCount(1);
176176
Block!.Images[0].Alt.Should().Be("Minimal carousel");
177177
Collector.Diagnostics.Count.Should().Be(0);

0 commit comments

Comments
 (0)