Address peer edits

Author: theletterf

src/Elastic.Documentation.Site/Assets/open-details-with-anchor.ts

@@ -6,14 +6,10 @@ export function openDetailsWithAnchor() {
         if (target) {
             const closestDetails = target.closest('details')
             if (closestDetails && !closestDetails.open) {
-                // Mark that we're doing a programmatic open
-                isProgrammaticOpen = true
-                closestDetails.open = true
-
-                // Reset the flag after the toggle event has fired
-                setTimeout(() => {
-                    isProgrammaticOpen = false
-                }, 10)
+                // Only open if it's not already open by default
+                if (!closestDetails.dataset.openDefault) {
+                    closestDetails.open = true
+                }
             }
 
             // Chrome automatically ensures parent content is visible, scroll immediately
@@ -38,9 +34,6 @@ function updateUrlForDropdown(details: HTMLDetailsElement, isOpening: boolean) {
     // This keeps the URL consistent with how headings behave
 }
 
-// Track if we're currently in a programmatic open operation
-let isProgrammaticOpen = false
-
 // Initialize the anchor handling functionality
 export function initOpenDetailsWithAnchor() {
     // Handle initial page load
@@ -49,23 +42,22 @@ export function initOpenDetailsWithAnchor() {
     // Handle hash changes within the same page (e.g., clicking anchor links)
     window.addEventListener('hashchange', openDetailsWithAnchor)
 
-    // Remove data-skip-url-update on first click to enable URL updates
+    // Remove data-open-default on first click to enable URL updates
     document.addEventListener(
         'click',
         (event) => {
             const target = event.target as HTMLElement
             const dropdown = target.closest(
                 'details.dropdown'
             ) as HTMLDetailsElement
-            if (dropdown && dropdown.dataset.skipUrlUpdate === 'true') {
-                delete dropdown.dataset.skipUrlUpdate
+            if (dropdown && dropdown.dataset.openDefault === 'true') {
+                delete dropdown.dataset.openDefault
             }
         },
         true
     )
 
     // Handle manual dropdown toggling to update URL
-    // Use event delegation to catch all toggle events
     document.addEventListener(
         'toggle',
         (event) => {
@@ -79,15 +71,12 @@ export function initOpenDetailsWithAnchor() {
                 const details = target as HTMLDetailsElement
                 const isOpening = details.open
 
-                // Only update URL if NOT skipping and NOT a programmatic open
-                if (!details.dataset.skipUrlUpdate && !isProgrammaticOpen) {
-                    // Use setTimeout to ensure the toggle state has been processed
-                    setTimeout(() => {
-                        updateUrlForDropdown(details, isOpening)
-                    }, 0)
+                // Only update URL if NOT open by default (until first interaction)
+                if (!details.dataset.openDefault) {
+                    updateUrlForDropdown(details, isOpening)
                 }
             }
         },
         true
-    ) // Use capture phase to ensure we catch the event
+    )
 }

src/Elastic.Markdown/Myst/Directives/Admonition/AdmonitionBlock.cs

@@ -47,8 +47,11 @@ public override void FinalizeAndValidate(ParserContext context)
 		// Auto-generate CrossReferenceName for dropdowns without explicit name
 		if (string.IsNullOrEmpty(CrossReferenceName) && (Admonition == "dropdown" || Classes == "dropdown"))
 		{
+			// Only generate a simple slug if no explicit name is provided
+			// This prevents ugly numbered anchors and forces content authors to be explicit about naming
+			// when they need linkable dropdowns
 			var baseSlug = Title.Slugify();
-			CrossReferenceName = context.GetUniqueSlug($"dropdown-{baseSlug}");
+			CrossReferenceName = baseSlug;
 		}
 	}
 }

src/Elastic.Markdown/Myst/Directives/Dropdown/DropdownView.cshtml

@@ -1,5 +1,8 @@
 @inherits RazorSlice<Elastic.Markdown.Myst.Directives.Admonition.AdmonitionViewModel>
-<details class="dropdown @Model.Classes" id="@Model.CrossReferenceName" open="@Model.Open" data-skip-url-update="@(Model.Open != null ? "true" : null)">
+<details class="dropdown @Model.Classes" 
+         id="@Model.CrossReferenceName" 
+         open="@Model.Open" 
+         data-open-default="@(Model.Open != null ? "true" : null)">
 	<summary class="dropdown-title">
 		<span class="sd-summary-text">@Model.Title</span>
 		<svg

src/Elastic.Markdown/Myst/ParserContext.cs

@@ -64,8 +64,6 @@ public class ParserContext : MarkdownParserContext, IParserResolvers
 	public IReadOnlyDictionary<string, string> Substitutions { get; }
 	public IReadOnlyDictionary<string, string> ContextSubstitutions { get; }
 
-	private readonly Dictionary<string, int> _slugCounters = [];
-
 	public ParserContext(ParserState state)
 	{
 		Build = state.Build;
@@ -112,16 +110,6 @@ public ParserContext(ParserState state)
 		ContextSubstitutions = contextSubs;
 	}
 
-	public string GetUniqueSlug(string baseSlug)
-	{
-		if (!_slugCounters.TryGetValue(baseSlug, out var count))
-		{
-			_slugCounters[baseSlug] = 1;
-			return baseSlug;
-		}
-
-		count++;
-		_slugCounters[baseSlug] = count;
-		return $"{baseSlug}-{count}";
-	}
+	// Removed GetUniqueSlug method - we don't want to automatically create unique slugs
+	// as this masks content organization issues and creates ugly, persistent anchors
 }

tests/Elastic.Markdown.Tests/FileSystemExtensionsTests.cs

@@ -13,7 +13,7 @@
 
 namespace Elastic.Markdown.Tests;
 
-public class FileSystemExtensionsTest(ITestOutputHelper output)
+public class FileSystemExtensionsTest
 {
 	[Fact]
 	public void IsSubPathOfTests()