-
Notifications
You must be signed in to change notification settings - Fork 410
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
Add member icons to navigation menu #2578
Conversation
Documentables with Java sources will have Java-styled icons, otherwise Kotlin-styled icons will be displayed. Also added handling of long text links and package names in the navigation tree, it's displayed properly now, with icon always on the left (see this class for instance) Current pre-review version for a mixed language project: https://ignatberesnev.github.io/static/dokka/icons/alchemist-java/alchemist-incarnation-biochemistry/it.unibo.alchemist.model.interfaces/-environment-supporting-deformable-cells/index.html |
@@ -191,56 +191,6 @@ class KotlinEnumsTest : BaseAbstractTest() { | |||
} | |||
} | |||
|
|||
@Test | |||
fun `should preserve enum source ordering for navigation menu`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum entries have been dropped from the navigation menu.
Also removed duplicated class KotlinEnumTest
- no differences in terms of tests
@@ -57,4 +68,50 @@ class JavaEnumsTest : BaseAbstractTest() { | |||
} | |||
} | |||
} | |||
|
|||
@Test | |||
fun `should mark synthetic functions generated for Kotlin as obvious`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some classes got duplicated for some reason, probably due to automatic conflict resolution. Deleted JavaEnumTest
and moved this test to JavaEnumsTest
, that was the only difference
import org.jetbrains.dokka.plugability.DokkaContext | ||
import org.jetbrains.dokka.transformers.pages.PageTransformer | ||
|
||
open class NavigationPageInstaller(val context: DokkaContext) : NavigationDataProvider(), PageTransformer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class was getting a bit too long for htmlPreprocessors.kt
, extracted it. chooseNavigationIcon
method is new
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NavigationPageInstaller
is a html preprocessor so it should be in htmlPreprocessors.kt
.
Extracted NavigationDataProvider
is ok .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it necessarily should be in the same file, sooner or later it'll grow so big that all of them will have to be extracted into their own ones. Imho, grouping classes by features rather than domain is easier for navigating around
Doesn't make a huge difference now, so returned it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern: If I open htmlPreprocessors.kt
, I expect to see ALL preprocessors there.
Imho, grouping classes by features rather than domain is easier for navigating around
Anyway now there are two different grouping simultaneously. It is misleading.
plugins/base/src/main/kotlin/renderers/html/NavigationDataProvider.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I am waiting source dependent icons.
And it would be great to combine the icons into a SVG sprite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked it in a mobile browser!?
plugins/base/src/main/kotlin/renderers/html/NavigationDataProvider.kt
Outdated
Show resolved
Hide resolved
import org.jetbrains.dokka.plugability.DokkaContext | ||
import org.jetbrains.dokka.transformers.pages.PageTransformer | ||
|
||
open class NavigationPageInstaller(val context: DokkaContext) : NavigationDataProvider(), PageTransformer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NavigationPageInstaller
is a html preprocessor so it should be in htmlPreprocessors.kt
.
Extracted NavigationDataProvider
is ok .
plugins/base/src/main/kotlin/renderers/html/NavigationDataProvider.kt
Outdated
Show resolved
Hide resolved
I did, it works and doesn't look worse than before. Is there something wrong with it? |
6855ae1
to
5df772d
Compare
No, I have not tested it there. |
Navigation icons to help distinguish between members. Styling is TBD
See #2576
Live example: https://ignatberesnev.github.io/static/dokka/icons/intellij/neutral-java-style/kotlinx-coroutines-core/kotlinx.coroutines/-global-scope/index.html