feat: enable optional Diagrams List Index

[galuszkak]

The idea is that when there is more than couple of diagrams on a given it's hard to scroll to find right one, or to link to exactly the one you are interested in on that page. To speed up this I've added just ordered list so you can quickly search based on index with links to the diagram.
This is by default turned off, so shouldn't affect existing users. To turn it on property generatr.site.listIndexViews has to be set to true

Example of the view:

[jenspav]

Thanks for the PR. Yes, I agree that navigation is cumbersome with multiple diagrams on the same page.

I haven't looked into the details, but already a few general remarks:

• I would prefer to not introduce a configuration option. What about always showing the index when there are more than one diagrams (or image views) to show, but not showing an index when there is only one diagram?
• What about using an unordered list instead of a numbered list? Otherwise it is seems that there is an order, but actually there isn't. Something like this:

    h6 {
        +"Jump to:"
    }
    ul {
        viewModel.diagrams.forEach {
            li {
                a(href="#${it.key}") {
                    +(it.title ?: it.name)
                }
            }
        }
    }

• From what I saw, image views are not yet taken into account. We should do this for both diagrams and image views.

[galuszkak]

Hi @jenspav,
Ad 1. Done. Removed configuration option and introduce this based on number of diagrams + images.
Ad 2. Done. Changed to unordered list.
Ad 3. Done. Added inclusion of images.

[jenspav]

Please add a similar test to other page tests as well, also with variations with/without diagrams and image views.

[galuszkak]

Done some initial tests on all pages. Probably we could do more, let me know if this would be enough or you need more.

[jenspav]

See above, would be nice to always test both scenarios. Also please adjust the test names to use the words index and visible instead of show and list .

[jenspav]

PS: Please do not use commit messages like "Reworked as requested by " since the context of the review is gone when looking later at the git history. Just name what you have changed and why. See https://cbea.ms/git-commit/ as a guideline.

[jenspav]

PPS: Thanks of course for your work on this!

[galuszkak]

PS: Please do not use commit messages like "Reworked as requested by " since the context of the review is gone when looking later at the git history. Just name what you have changed and why. See https://cbea.ms/git-commit/ as a guideline.

Should I add this into CONTRIBUTING.md file?

[galuszkak]

I think I've addressed most feedback on this PR. Please review again changes + comments @jenspav.

Thanks for Your help!

[jenspav]

The file should be named DiagramIndex.kt.

[jenspav]

We should test both scenarios, like this:

    @Test
    fun `no index`() {
        val viewModel = SoftwareSystemContainerComponentCodePageViewModel(generatorContext, backendContainer, backendComponent)
        assertThat(viewModel.diagramIndex.visible).isFalse()
    }

    @Test
    fun `index visible`() {
        createImageView(generatorContext.workspace, backendComponent, "other-imageview")
        val viewModel = SoftwareSystemContainerComponentCodePageViewModel(generatorContext, backendContainer, backendComponent)

        assertThat(viewModel.diagramIndex.visible).isTrue()
    }

[jenspav]

See above, would be nice to always test both scenarios. Also please adjust the test names to use the words index and visible instead of show and list .

[jenspav]

Feel free to use this patch since I did some adjustments locally to see how it fits:

diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/DiagramIndexViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/DiagramIndexViewModel.kt
index cea2d12..4c1f072 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/DiagramIndexViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/DiagramIndexViewModel.kt
@@ -3,6 +3,6 @@ package nl.avisi.structurizr.site.generatr.site.model
 
 data class DiagramIndexViewModel(
     val diagrams: List<DiagramViewModel>,
-    val images: List<ImageViewViewModel>,
-    val showList: Boolean = (diagrams.count() + images.count()) > 1
-)
+    val images: List<ImageViewViewModel>) {
+    val visible: Boolean = (diagrams.count() + images.count()) > 1
+}
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModel.kt
index 2f66d81..57d2438 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModel.kt
@@ -16,10 +16,7 @@ class SoftwareSystemContainerComponentCodePageViewModel(generatorContext: Genera
     val visible = images.isNotEmpty()
     val containerTabs = createContainersCodeTabViewModel(generatorContext, container.softwareSystem)
     val componentTabs = createComponentsTabViewModel(generatorContext, container)
-    val diagramIndex = DiagramIndexViewModel(
-        emptyList(),
-        images
-    )
+    val diagramIndex = DiagramIndexViewModel(emptyList(), images)
     companion object {
         fun url(container: Container, component: Component?) = "${url(container.softwareSystem, Tab.CODE)}/${container.name.normalize()}/${component?.name?.normalize()}"
     }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModel.kt
index 32bbf6e..9cbbe8f 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModel.kt
@@ -21,10 +21,7 @@ class SoftwareSystemContainerComponentsPageViewModel(generatorContext: Generator
     val propertiesTable = createPropertiesTableViewModel(container.includedProperties)
     val visible = diagrams.isNotEmpty() or images.isNotEmpty() or hasProperties
     val containerTabs = createContainersComponentTabViewModel(generatorContext, container.softwareSystem)
-    val diagramIndex = DiagramIndexViewModel(
-        diagrams,
-        images
-    )
+    val diagramIndex = DiagramIndexViewModel(diagrams, images)
     companion object {
         fun url(container: Container) = "${url(container.softwareSystem, Tab.COMPONENT)}/${container.name.normalize()}"
     }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModel.kt
index a0ea3ff..8db1803 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModel.kt
@@ -15,8 +15,5 @@ class SoftwareSystemContainerPageViewModel(generatorContext: GeneratorContext, s
         .sortedBy { it.key }
         .map { ImageViewViewModel(it) }
     val visible = generatorContext.workspace.views.hasContainerViews(generatorContext.workspace, softwareSystem) || images.isNotEmpty()
-    val diagramIndex = DiagramIndexViewModel(
-        diagrams,
-        images
-    )
+    val diagramIndex = DiagramIndexViewModel(diagrams, images)
 }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModel.kt
index 396984b..075c4ce 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModel.kt
@@ -11,8 +11,5 @@ class SoftwareSystemContextPageViewModel(generatorContext: GeneratorContext, sof
         .sortedBy { it.key }
         .map { DiagramViewModel.forView(this, it, generatorContext.svgFactory) }
     val visible = generatorContext.workspace.views.hasSystemContextViews(softwareSystem)
-    val diagramIndex = DiagramIndexViewModel(
-        diagrams,
-        emptyList()
-    )
+    val diagramIndex = DiagramIndexViewModel(diagrams, emptyList())
 }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModel.kt
index 026707c..fd931c1 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModel.kt
@@ -11,8 +11,5 @@ class SoftwareSystemDeploymentPageViewModel(generatorContext: GeneratorContext,
         .sortedBy { it.key }
         .map { DiagramViewModel.forView(this, it, generatorContext.svgFactory) }
     val visible = generatorContext.workspace.views.hasDeploymentViews(softwareSystem)
-    val diagramIndex = DiagramIndexViewModel(
-        diagrams,
-        emptyList()
-    )
+    val diagramIndex = DiagramIndexViewModel(diagrams, emptyList())
 }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModel.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModel.kt
index 6d32864..3fa5477 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModel.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModel.kt
@@ -11,8 +11,5 @@ class SoftwareSystemDynamicPageViewModel(generatorContext: GeneratorContext, sof
         .sortedBy { it.key }
         .map { DiagramViewModel.forView(this, it, generatorContext.svgFactory) }
     val visible = generatorContext.workspace.views.hasDynamicViews(softwareSystem)
-    val diagramIndex = DiagramIndexViewModel(
-        diagrams,
-        emptyList()
-    )
+    val diagramIndex = DiagramIndexViewModel(diagrams, emptyList())
 }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/DiagramIndexList.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/DiagramIndexList.kt
index b1824b5..940f2f5 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/DiagramIndexList.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/DiagramIndexList.kt
@@ -4,22 +4,22 @@ import kotlinx.html.*
 import nl.avisi.structurizr.site.generatr.site.model.DiagramIndexViewModel
 
 fun FlowContent.diagramIndex(viewModel: DiagramIndexViewModel) {
-    if(viewModel.showList) {
-        h5 {
+    if(viewModel.visible) {
+        h6 {
             +"Jump to: "
         }
         ul {
-            viewModel.diagrams?.forEach {
+            viewModel.diagrams.forEach {
                 li {
                     a(href = "#${it.key}") {
                         +(it.title ?: it.name)
                     }
                 }
             }
-            viewModel.images?.forEach {
+            viewModel.images.forEach {
                 li {
                     a(href = "#${it.key}") {
-                        +(it.title ?: it.name)
+                        +it.title
                     }
                 }
             }
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDeploymentPage.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDeploymentPage.kt
index 7590203..7636b0b 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDeploymentPage.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDeploymentPage.kt
@@ -7,9 +7,7 @@ fun HTML.softwareSystemDeploymentPage(viewModel: SoftwareSystemDeploymentPageVie
     if (viewModel.visible)
         softwareSystemPage(viewModel) {
             diagramIndex(viewModel.diagramIndex)
-            viewModel.diagrams.forEach {
-                diagram(it)
-            }
+            viewModel.diagrams.forEach { diagram(it) }
         }
     else
         redirectUpPage()
diff --git a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDynamicPage.kt b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDynamicPage.kt
index 35c51ef..b0a8f43 100644
--- a/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDynamicPage.kt
+++ b/src/main/kotlin/nl/avisi/structurizr/site/generatr/site/views/SoftwareSystemDynamicPage.kt
@@ -7,9 +7,7 @@ fun HTML.softwareSystemDynamicPage(viewModel: SoftwareSystemDynamicPageViewModel
     if (viewModel.visible)
         softwareSystemPage(viewModel) {
             diagramIndex(viewModel.diagramIndex)
-            viewModel.diagrams.forEach {
-                diagram(it)
-            }
+            viewModel.diagrams.forEach { diagram(it) }
         }
     else
         redirectUpPage()
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModelTest.kt
index ab82cf4..b48bdd8 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentCodePageViewModelTest.kt
@@ -109,10 +109,16 @@ class SoftwareSystemContainerComponentCodePageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is disabled`() {
+    fun `no index`() {
         val viewModel = SoftwareSystemContainerComponentCodePageViewModel(generatorContext, backendContainer, backendComponent)
-        assertThat(viewModel.diagramIndex.showList).isFalse()
+        assertThat(viewModel.diagramIndex.visible).isFalse()
+    }
 
+    @Test
+    fun `index visible`() {
+        createImageView(generatorContext.workspace, backendComponent, "other-imageview")
+        val viewModel = SoftwareSystemContainerComponentCodePageViewModel(generatorContext, backendContainer, backendComponent)
 
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModelTest.kt
index 39168c4..14a8d41 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerComponentsPageViewModelTest.kt
@@ -150,8 +150,8 @@ class SoftwareSystemContainerComponentsPageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is enabled`() {
+    fun `has index`() {
         val viewModel = SoftwareSystemContainerComponentsPageViewModel(generatorContext, backendContainer)
-        assertThat(viewModel.diagramIndex.showList).isTrue()
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModelTest.kt
index b1c2527..b6fa998 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContainerPageViewModelTest.kt
@@ -77,8 +77,8 @@ class SoftwareSystemContainerPageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is enabled`() {
+    fun `has index`() {
         val viewModel = SoftwareSystemContainerPageViewModel(generatorContext, softwareSystem)
-        assertThat(viewModel.diagramIndex.showList).isTrue()
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModelTest.kt
index eb868e3..45a49a9 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemContextPageViewModelTest.kt
@@ -56,8 +56,8 @@ class SoftwareSystemContextPageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is enabled`() {
+    fun `has index`() {
         val viewModel = SoftwareSystemContextPageViewModel(generatorContext, softwareSystem)
-        assertThat(viewModel.diagramIndex.showList).isTrue()
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModelTest.kt
index f3c08cd..53eb44f 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDeploymentPageViewModelTest.kt
@@ -65,8 +65,8 @@ class SoftwareSystemDeploymentPageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is enabled`() {
+    fun `has index`() {
         val viewModel = SoftwareSystemDeploymentPageViewModel(generatorContext, softwareSystem)
-        assertThat(viewModel.diagramIndex.showList).isTrue()
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModelTest.kt
index ac68974..304ca4c 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/SoftwareSystemDynamicPageViewModelTest.kt
@@ -67,8 +67,8 @@ class SoftwareSystemDynamicPageViewModelTest : ViewModelTest() {
     }
 
     @Test
-    fun `show list is enabled`() {
+    fun `has index`() {
         val viewModel = SoftwareSystemDynamicPageViewModel(generatorContext, softwareSystem)
-        assertThat(viewModel.diagramIndex.showList).isTrue()
+        assertThat(viewModel.diagramIndex.visible).isTrue()
     }
 }
diff --git a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/ViewModelTest.kt b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/ViewModelTest.kt
index d67b476..063da6c 100644
--- a/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/ViewModelTest.kt
+++ b/src/test/kotlin/nl/avisi/structurizr/site/generatr/site/model/ViewModelTest.kt
@@ -41,7 +41,7 @@ abstract class ViewModelTest {
 
     protected fun createSection(content: String = "# Content") = Section(Format.Markdown, content)
 
-    protected fun createImageView(workspace: Workspace, element: Element): ImageView = workspace.views.createImageView(element, "imageview-${element.id}").also {
+    protected fun createImageView(workspace: Workspace, element: Element, key:String = "imageview-${element.id}"): ImageView = workspace.views.createImageView(element, key).also {
         it.description = "Image View Description"
         it.title = "Image View Title"
         it.contentType = "image/png"

[galuszkak]

Thanks @jenspav, I'm not daily Kotlin user - so all feedback is greatly appreciated. I've applied the patch.

[kristoflagae]

Hi,
As an addition to the index functionality, it would be helpful to have a subtitle for each diagram.
The issue I'm facing is that some diagrams are so large (containing many elements), that the title on top of the diagram becomes too small to read without opening the diagram.

Also, the order of the displayed diagrams doesn't always match the order in the DSL file. Am I correct?

[jenspav]

@galuszkak We are getting there. There is one file rename left (see the still open comment) and I would like to see both scenarios (no index and index visible) tested for each test.

[jenspav]

Hi, As an addition to the index functionality, it would be helpful to have a subtitle for each diagram. The issue I'm facing is that some diagrams are so large (containing many elements), that the title on top of the diagram becomes too small to read without opening the diagram.

Also, the order of the displayed diagrams doesn't always match the order in the DSL file. Am I correct?

You can better open a new issue with e.g. a screenshot that visualizes your issue. Your scenario will get lost here.