Wrap long signatures dynamically based on client width (#2659)

8 files changed, 133 insertions, 126 deletions

diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
index dfc4e2e3..d949d432 100644
--- a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
@@ -112,11 +112,6 @@ open class HtmlRenderer(
             }
             node.dci.kind == SymbolContentKind.Parameter -> {
                 span("parameter $additionalClasses") {
-                    if (node.hasStyle(ContentStyle.Indented)) {
-                        // could've been done with CSS (padding-left, ::before, etc), but the indent needs to
-                        // consist of physical spaces, otherwise select and copy won't work properly
-                        repeat(4) { consumer.onTagContentEntity(Entities.nbsp) }
-                    }
                     childrenCallback()
                 }
             }
diff --git a/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt b/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
index 2de6f2b7..e2be3ef0 100644
--- a/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
@@ -45,12 +45,20 @@ class CustomResourceInstaller(val dokkaContext: DokkaContext) : PageTransformer
 }
 
 class ScriptsInstaller(private val dokkaContext: DokkaContext) : PageTransformer {
+
+    // scripts ending with `_deferred.js` are loaded with `defer`, otherwise `async`
     private val scriptsPages = listOf(
         "scripts/clipboard.js",
         "scripts/navigation-loader.js",
         "scripts/platform-content-handler.js",
         "scripts/main.js",
-        "scripts/prism.js"
+        "scripts/prism.js",
+
+        // It's important for this script to be deferred because it has logic that makes decisions based on
+        // rendered elements (for instance taking their clientWidth), and if not all styles are loaded/applied
+        // at the time of inspecting them, it will give incorrect results and might lead to visual bugs.
+        // should be easy to test if you open any page in incognito or by reloading it (Ctrl+Shift+R)
+        "scripts/symbol-parameters-wrapper_deferred.js",
     )
 
     override fun invoke(input: RootPageNode): RootPageNode =
diff --git a/plugins/base/src/main/kotlin/renderers/html/innerTemplating/DefaultTemplateModelFactory.kt b/plugins/base/src/main/kotlin/renderers/html/innerTemplating/DefaultTemplateModelFactory.kt
index 9f1ca57e..6c746b37 100644
--- a/plugins/base/src/main/kotlin/renderers/html/innerTemplating/DefaultTemplateModelFactory.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/innerTemplating/DefaultTemplateModelFactory.kt
@@ -102,7 +102,7 @@ class DefaultTemplateModelFactory(val context: DokkaContext) : TemplateModelFact
                             type = ScriptType.textJavaScript,
                             src = if (it.isAbsolute) it else "$pathToRoot$it"
                         ) {
-                            if (it == "scripts/main.js")
+                            if (it == "scripts/main.js" || it.endsWith("_deferred.js"))
                                 defer = true
                             else
                                 async = true
@@ -204,4 +204,4 @@ private class TemplateDirective(val configuration: DokkaConfiguration, val pathT
         const val PARAM_NAME = "name"
         const val PARAM_REPLACEMENT = "replacement"
     }
-}
\ No newline at end of file
+}
diff --git a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
index 7ed7ff3f..f6c4f0db 100644
--- a/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
+++ b/plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
@@ -202,39 +202,18 @@ interface JvmSignatureUtils {
     fun PageContentBuilder.DocumentableContentBuilder.parametersBlock(
         function: DFunction, paramBuilder: PageContentBuilder.DocumentableContentBuilder.(DParameter) -> Unit
     ) {
-        val shouldWrap = function.shouldWrapParams()
-        val parametersStyle = if (shouldWrap) setOf(ContentStyle.Wrapped) else emptySet()
-        val elementStyle = if (shouldWrap) setOf(ContentStyle.Indented) else emptySet()
-        group(kind = SymbolContentKind.Parameters, styles = parametersStyle) {
+        group(kind = SymbolContentKind.Parameters, styles = emptySet()) {
             function.parameters.dropLast(1).forEach {
-                group(kind = SymbolContentKind.Parameter, styles = elementStyle) {
+                group(kind = SymbolContentKind.Parameter) {
                     paramBuilder(it)
                     punctuation(", ")
                 }
             }
-            group(kind = SymbolContentKind.Parameter, styles = elementStyle) {
+            group(kind = SymbolContentKind.Parameter) {
                 paramBuilder(function.parameters.last())
             }
         }
     }
-
-    /**
-     * Determines whether parameters in a function (including constructor) should be wrapped
-     *
-     * Without wrapping:
-     * ```
-     * class SimpleClass(foo: String, bar: String) {}
-     * ```
-     * With wrapping:
-     * ```
-     * class SimpleClass(
-     *     foo: String,
-     *     bar: String,
-     *     baz: String
-     * )
-     * ```
-     */
-    private fun DFunction.shouldWrapParams() = this.parameters.size >= 3
 }
 
 sealed class AtStrategy
diff --git a/plugins/base/src/main/resources/dokka/scripts/symbol-parameters-wrapper_deferred.js b/plugins/base/src/main/resources/dokka/scripts/symbol-parameters-wrapper_deferred.js
new file mode 100644
index 00000000..248d0ab0
--- /dev/null
+++ b/plugins/base/src/main/resources/dokka/scripts/symbol-parameters-wrapper_deferred.js
@@ -0,0 +1,83 @@
+// helps with some corner cases where <wbr> starts working already,
+// but the signature is not yet long enough to be wrapped
+const leftPaddingPx = 60
+
+const symbolResizeObserver = new ResizeObserver(entries => {
+    entries.forEach(entry => {
+        const symbolElement = entry.target
+        symbolResizeObserver.unobserve(symbolElement) // only need it once, otherwise will be executed multiple times
+        wrapSymbolParameters(symbolElement);
+    })
+});
+
+const wrapAllSymbolParameters = () => {
+    document.querySelectorAll("div.symbol").forEach(symbol => wrapSymbolParameters(symbol))
+}
+
+const wrapSymbolParameters = (symbol) => {
+    let parametersBlock = symbol.querySelector("span.parameters")
+    if (parametersBlock == null) {
+        return // nothing to wrap
+    }
+
+    let symbolBlockWidth = symbol.clientWidth
+
+    // Even though the script is marked as `defer` and we wait for `DOMContentLoaded` event,
+    // it can happen that `symbolBlockWidth` is 0, indicating that something hasn't been loaded.
+    // In this case, just retry once all styles have been applied and it has been resized correctly.
+    if (symbolBlockWidth === 0) {
+        symbolResizeObserver.observe(symbol)
+        return
+    }
+
+    let innerTextWidth = Array.from(symbol.children)
+        .filter(it => !it.classList.contains("block")) // blocks are usually on their own (like annotations), so ignore it
+        .map(it => it.getBoundingClientRect().width).reduce((a, b) => a + b, 0)
+
+    // if signature text takes up more than a single line, wrap params for readability
+    let shouldWrapParams = innerTextWidth > (symbolBlockWidth - leftPaddingPx)
+    if (shouldWrapParams) {
+        parametersBlock.classList.add("wrapped")
+        parametersBlock.querySelectorAll("span.parameter").forEach(param => {
+            // has to be a physical indent so that it can be copied. styles like
+            // paddings and `::before { content: "    " }` do not work for that
+            param.prepend(createNbspIndent())
+        })
+    }
+}
+
+const createNbspIndent = () => {
+    let indent = document.createElement("span")
+    indent.append(document.createTextNode("\u00A0\u00A0\u00A0\u00A0"))
+    indent.classList.add("nbsp-indent")
+    return indent
+}
+
+const resetAllSymbolParametersWrapping = () => {
+    document.querySelectorAll("div.symbol").forEach(symbol => resetSymbolParametersWrapping(symbol))
+}
+
+const resetSymbolParametersWrapping = (symbol) => {
+    let parameters = symbol.querySelector("span.parameters")
+    if (parameters != null) {
+        parameters.classList.remove("wrapped")
+        parameters.querySelectorAll("span.parameter").forEach(param => {
+            let indent = param.querySelector("span.nbsp-indent")
+            if (indent != null) indent.remove()
+        })
+    }
+}
+
+if (document.readyState === 'loading') {
+    window.addEventListener('DOMContentLoaded', () => {
+        wrapAllSymbolParameters()
+    })
+} else {
+    wrapAllSymbolParameters()
+}
+
+window.onresize = event => {
+    // need to re-calculate if params need to be wrapped after resize
+    resetAllSymbolParametersWrapping()
+    wrapAllSymbolParameters()
+}
diff --git a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
index 61c9556e..13e103b4 100644
--- a/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
+++ b/plugins/base/src/test/kotlin/signatures/SignatureTest.kt
@@ -850,53 +850,6 @@ class SignatureTest : BaseAbstractTest() {
     }
 
     @Test
-    fun `fun with single param should NOT have any wrapped or indented parameters`() {
-        val source = source("fun assertNoIndent(int: Int): String = \"\"")
-        val writerPlugin = TestOutputWriterPlugin()
-
-        testInline(
-            source,
-            configuration,
-            pluginOverrides = listOf(writerPlugin)
-        ) {
-            renderingStage = { _, _ ->
-                val signature = writerPlugin.writer.renderedContent("root/example/assert-no-indent.html").firstSignature()
-                signature.match(
-                    "fun", A("assertNoIndent"), "(", Parameters(
-                        Parameter("int: ", A("Int")),
-                    ), "): ", A("String"),
-                    ignoreSpanWithTokenStyle = true
-                )
-                assertFalse { signature.select("span.parameters").single().hasClass("wrapped") }
-                assertFalse { signature.select("span.parameters > span.parameter").single().hasClass("indented") }
-            }
-        }
-    }
-
-    @Test
-    fun `fun with many params should have wrapped and indented parameters`() {
-        val source = source("fun assertParamsIndent(int: Int, string: String, long: Long): String = \"\"")
-        val writerPlugin = TestOutputWriterPlugin()
-
-        testInline(
-            source,
-            configuration,
-            pluginOverrides = listOf(writerPlugin)
-        ) {
-            renderingStage = { _, _ ->
-                writerPlugin.writer.renderedContent("root/example/assert-params-indent.html").firstSignature().match(
-                    "fun", A("assertParamsIndent"), "(", Parameters(
-                        Parameter("int: ", A("Int"), ",").withClasses("indented"),
-                        Parameter("string: ", A("String"), ",").withClasses("indented"),
-                        Parameter("long: ", A("Long")).withClasses("indented")
-                    ).withClasses("wrapped"), "): ", A("String"),
-                    ignoreSpanWithTokenStyle = true
-                )
-            }
-        }
-    }
-
-    @Test
     fun `const val with default values`() {
         val source = source("const val simpleVal = 1")
         val writerPlugin = TestOutputWriterPlugin()
diff --git a/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt b/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt
index 8685e3c9..67e4d16f 100644
--- a/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt
+++ b/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt
@@ -8,7 +8,10 @@ import org.jetbrains.dokka.transformers.pages.PageTransformer
 import org.jetbrains.dokka.transformers.pages.pageMapper
 import org.jetbrains.dokka.transformers.pages.pageScanner
 import org.jetbrains.dokka.transformers.pages.pageStructureTransformer
+import org.jsoup.Jsoup
 import org.junit.jupiter.api.Test
+import utils.TestOutputWriterPlugin
+import utils.assertContains
 import kotlin.test.assertEquals
 
 class PageTransformerBuilderTest : BaseAbstractTest() {
@@ -168,6 +171,39 @@ class PageTransformerBuilderTest : BaseAbstractTest() {
         }
     }
 
+    @Test
+    fun `should load script as defer if name ending in _deferred`() {
+        val configuration = dokkaConfiguration {
+            sourceSets {
+                sourceSet {
+                    sourceRoots = listOf("src/main/kotlin")
+                }
+            }
+        }
+        val writerPlugin = TestOutputWriterPlugin()
+        testInline(
+            """
+            |/src/main/kotlin/test/Test.kt
+            |package test
+            |
+            |class Test
+        """.trimMargin(),
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            renderingStage = { _, _ ->
+                val generatedFiles = writerPlugin.writer.contents
+
+                assertContains(generatedFiles.keys, "scripts/symbol-parameters-wrapper_deferred.js")
+
+                val scripts = generatedFiles.getValue("root/test/-test/-test.html").let { Jsoup.parse(it) }.select("script")
+                val deferredScriptSources = scripts.filter { it.hasAttr("defer") }.map { it.attr("src") }
+
+                // important to check symbol-parameters-wrapper_deferred specifically since it might break some features
+                assertContains(deferredScriptSources, "../../../scripts/symbol-parameters-wrapper_deferred.js")
+            }
+        }
+    }
 
     private fun <T> Collection<T>.assertCount(n: Int, prefix: String = "") =
         assert(count() == n) { "${prefix}Expected $n, got ${count()}" }
diff --git a/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt b/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt
index 6b7a2ae0..b43cea05 100644
--- a/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt
+++ b/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt
@@ -489,53 +489,6 @@ class KotlinAsJavaPluginTest : BaseAbstractTest() {
         }
     }
 
-    @Test
-    fun `should add wrapping and indent to parameters if too many`() {
-        val writerPlugin = TestOutputWriterPlugin()
-        val configuration = dokkaConfiguration {
-            sourceSets {
-                sourceSet {
-                    sourceRoots = listOf("src/")
-                    externalDocumentationLinks = listOf(
-                        DokkaConfiguration.ExternalDocumentationLink.jdk(8),
-                        stdlibExternalDocumentationLink
-                    )
-                }
-            }
-        }
-        testInline(
-            """
-            |/src/main/kotlin/kotlinAsJavaPlugin/Wrapped.kt
-            |package kotlinAsJavaPlugin
-            |
-            |class Wrapped(val xd: Int, val l: Long, val s: String)
-        """.trimMargin(),
-            configuration,
-            pluginOverrides = listOf(writerPlugin),
-            cleanupOutput = true
-        ) {
-            pagesGenerationStage = { root ->
-                val content = root.children
-                    .flatMap { it.children<ContentPage>() }
-                    .map { it.content }.single().mainContents
-
-                val text = content.single { it is ContentHeader }.children
-                    .single() as ContentText
-
-                assertEquals("Constructors", text.text)
-            }
-            renderingStage = { _, _ ->
-                writerPlugin.writer.renderedContent("root/kotlinAsJavaPlugin/-wrapped/-wrapped.html").firstSignature().match(
-                    A("Wrapped"), A("Wrapped"), "(", Parameters(
-                        Parameter(A("Integer"), "xd,").withClasses("indented"),
-                        Parameter(A("Long"), "l,").withClasses("indented"),
-                        Parameter(A("String"), "s").withClasses("indented"),
-                    ).withClasses("wrapped"), ")", ignoreSpanWithTokenStyle = true
-                )
-            }
-        }
-    }
-
     /**
      * Kotlin Int becomes java int. Java int cannot be annotated in source, but Kotlin Int can be.
      * This is paired with DefaultDescriptorToDocumentableTranslatorTest.`Java primitive annotations work`()