Skip to content

Commit

Permalink
Wrap long signatures dynamically based on client width (#2659)
Browse files Browse the repository at this point in the history
  • Loading branch information
IgnatBeresnev committed Sep 21, 2022
1 parent e49726f commit 879d4f2
Show file tree
Hide file tree
Showing 8 changed files with 133 additions and 126 deletions.
5 changes: 0 additions & 5 deletions plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
Expand Up @@ -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()
}
}
Expand Down
Expand Up @@ -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 =
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -204,4 +204,4 @@ private class TemplateDirective(val configuration: DokkaConfiguration, val pathT
const val PARAM_NAME = "name"
const val PARAM_REPLACEMENT = "replacement"
}
}
}
27 changes: 3 additions & 24 deletions plugins/base/src/main/kotlin/signatures/JvmSignatureUtils.kt
Expand Up @@ -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
Expand Down
@@ -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()
}
47 changes: 0 additions & 47 deletions plugins/base/src/test/kotlin/signatures/SignatureTest.kt
Expand Up @@ -849,53 +849,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")
Expand Down
Expand Up @@ -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() {
Expand Down Expand Up @@ -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()}" }
Expand Down
47 changes: 0 additions & 47 deletions plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt
Expand Up @@ -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`()
Expand Down

0 comments on commit 879d4f2

Please sign in to comment.