Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap long signatures dynamically based on taken client width #2659

Merged
merged 5 commits into from Sep 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
IgnatBeresnev marked this conversation as resolved.
Show resolved Hide resolved
.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