avisi-cloud / structurizr-site-generatr

Static site generator for architecture models created with Structurizr DSL
https://avisi-cloud.github.io/structurizr-site-generatr/
Apache License 2.0
214 stars 30 forks source link

Theme behaviour needs attention #40

Closed julrichkieffer closed 1 year ago

julrichkieffer commented 2 years ago

Problem

This generator applies the same theme (it appears) regardless of the DSL default styles or theme / themes grammar. Consequently, workspaces always generate the same look-and-feel to diagrams regardless of any settings in the .dsl file.

Triage

To triage this behaviour, the following DSL is used:

workspace "multi-relationships-summary" {

    model {
        cpq = softwareSystem "CPQ" "" "Web Browser"

        admin = person "Admin" {
            -> cpq "administers"
        }
        user = person "CPQ User" "" "Bank Staff" {
            -> cpq "uses"
        }
    }

    views {

        # theme default
        # themes default https://structurizr.com/share/36141/theme

        systemLandscape {
            autoLayout
            // show all static relationships and entities (see model)
            include *
        }

        dynamic * "CPQAdmin" {
            autoLayout
            title "CPQ Administration"

            // include static relationship; no change to relationship as only the parties are identified
            user -> cpq

            // replace static relationship; parties are identified but new relationships (both directions) are shown in this view only
            admin -> cpq "maintains products"
            admin -> cpq "maintains pricing"
            admin -> cpq "maintains promotions"
            admin -> cpq "maintains cross-selling"
            cpq -> admin "report abandonned carts"
            cpq -> admin "report sales overrides"
        }
    }
}
DSL / Workspace setting Expected Actual Notes
none out-the-box expected out-the-box actual While applying a theme when none is specified is welcome, a way to opt out of this is needed. By opting out, the Structurizr out-the-box theme (illustrated here) should be applied instead.
theme default default expected default actual When a theme is specified, it should be applied. Here the default value refers to the Structurizr default theme
themes default https://structurizr.com/share/36141/theme multiple expected multiple actual When a theme is specified, it should be applied. Here the default value refers to the Structurizr default theme. Subsequent themes are applied in order, like CSS.

Expected results are taken from Structurizr DSL Viewer. Actual results are taken by running this generator against the same .dsl workspace, and retrieving the .png from the /build output.

simonbrowndotje commented 2 years ago

It looks like you're using the C4PlantUMLExporter, which uses it's own styling. Perhaps try the StructurizrPlantUMLExporter or MermaidDiagramExporter instead.

You can also fork the C4PlantUMLExporter if needed.

dirkgroot commented 2 years ago

Hi @simonbrowndotje, thanks for your suggestion! I've tried using StructurizrPlantUMLExporter instead of C4PlantUMLExporter. However, StructurizrPlantUMLExporter doesn't seem to support themes. It does support styles (at least to some degree, I've tested it with Big Bank plc workspace, which includes some styles).

I prefer that this tool just uses the Structurizr libraries "out of the box", so I don't really mind if there are some limitations when it comes to available features.

So when it comes to supporting themes, I think the best solution would be to add support for theme[s] to StructurizrPlantUMLExporter and then add a command-line option to this tool, so the user can decide which diagram style they prefer:

simonbrowndotje commented 2 years ago

Themes are supported, but you'll need to call the following method to load them into the workspace before running the export:

ThemeUtils.loadThemes(workspace);
dirkgroot commented 2 years ago

Awesome, that works, thanks!

Just double checking: As far as I can tell, support for themes/styles is very limited when using C4PlantUMLExporter. By looking at the source code, I only see that it supports customization of the shape of a container, to be either a cylinder or a pipe. Am I missing something?

simonbrowndotje commented 2 years ago

support for themes/styles is very limited when using C4PlantUMLEpxorter

Correct ... C4-PlantUML was designed to use it's own styling, and while you can override this with tags, that's not supported by the C4PlantUMLEpxorter. I have no plans to add this functionality, but there's a fork that already does this -> https://github.com/cloudflightio/structurizr-export-c4plantuml

dirkgroot commented 2 years ago

Okay, thanks!

I think the best way to approach this, is to support (a subset of) the diagram formats supported by export command of the Structurizr CLI:

> structurizr-cli export
Missing required options: w, f
usage: export
 -f,--format <arg>      Export format: plantuml[/structurizr|c4plantuml]|websequencediagrams|mermaid|dot|ilograph|json|dsl|theme|fqcn
 -o,--output <arg>      Path to an output directory
 -w,--workspace <arg>   Path or URL to the workspace JSON file/DSL file(s)

A good start would be to support plantuml/structurizr and plantuml/c4plantuml.

jp7677 commented 1 year ago

~Related and from my experiences the actual "short-term" blocking issue for better customization: https://github.com/structurizr/export/issues/28~

Edit: This issue has been closed as completed.

jp7677 commented 1 year ago

Setting alternative colors and shapes seems to work just fine for me now when using tags i.c.w. with the latest version:

Click to show DLS ``` views { properties { "c4plantuml.tags" "true" } ... styles { element "Person" { color #ffffff fontSize 22 shape Person } element "Customer" { background #997662 } element "Bank Staff" { background #137baa } element "Software System" { background #8ba65d color #ffffff } element "Existing System" { background #abb39d color #000000 } ... } ... } ```

Results in:

Click to show Diagram ![Screenshot from 2023-01-17 10-08-01](https://user-images.githubusercontent.com/17219293/212855952-f63ac8df-4312-4b76-9fd2-40eff5fe2791.png)

(Please don't look to much at those random colors, I'm not a designer.. ;))

So at least from my side this is resolved. But not sure if i hit the same use case as the OP.

dirkgroot commented 1 year ago

Thanks @jp7677! I've created a PR #125 which makes sure that themes are loaded when loading a Structurizr DSL workspace. With this change, the output of the DSL from the OP looks like this, after setting the property c4plantuml.tags to true:

With no theme set ![image](https://user-images.githubusercontent.com/3872163/213123941-a8c6bdaa-55d2-47a4-8bc1-4e215bbf5700.png)
With theme default ![image](https://user-images.githubusercontent.com/3872163/213124042-662ea70e-eec0-47f1-85c4-1f32528a0746.png)
With themes default https://structurizr.com/share/36141/theme ![image](https://user-images.githubusercontent.com/3872163/213124228-1bfa4faf-eef2-4588-83b9-2679e2d11295.png)
With themes https://structurizr.com/share/36141/theme ![image](https://user-images.githubusercontent.com/3872163/213124359-82a7bc39-b114-4757-8322-d92408a5f132.png)

I think this is very close to what @julrichkieffer expects, except for the case themes default https://structurizr.com/share/36141/theme.

@simonbrowndotje Could it be that StructurizrPlantUMLExporter handles multiple themes differently than C4PlantUMLExporter?

simonbrowndotje commented 1 year ago

Could it be that StructurizrPlantUMLExporter handles multiple themes differently than C4PlantUMLExporter?

Yes, although the gap between how the two exporters handle styles has narrowed since this issue was opened:

dirkgroot commented 1 year ago

Yes, although the gap between how the two exporters handle styles has narrowed since this issue was opened:

Okay, good to know, thanks!

dirkgroot commented 1 year ago

Now that C4PlantUMLExporter has been adjusted to better support themes, and #125 is merged, I'm closing this issue. With these changes, the actual behaviour is almost the same as the expected behaviour @julrichkieffer describes.