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

Structurizr: Cannot override styles when using default theme #1586

Closed
bdabelow opened this issue Jun 26, 2023 · 3 comments · Fixed by #1592
Closed

Structurizr: Cannot override styles when using default theme #1586

bdabelow opened this issue Jun 26, 2023 · 3 comments · Fixed by #1592
Labels
🐞 bug Something isn't working

Comments

@bdabelow
Copy link

bdabelow commented Jun 26, 2023

Thanks a bunch for kroki, it makes my life a lot easier.

Small issue I have: this example fails with 0.21.0 and later, but works with 0.20.0:

workspace {

    model {
        user = person "User" "A user of my software system."
        softwareSystem = softwareSystem "Software System" "My software system."

        user -> softwareSystem "Uses"
    }

    views {
        theme default

        systemContext softwareSystem "SystemContext" {
            include *
            autoLayout
        }

        styles {
            element "Software System" {
                background #1168bd
                color #ffffff
            }
        }
    }
    
}
{
  "timestamp": "1687778933416",
  "level": "ERROR",
  "thread": "vert.x-eventloop-thread-1",
  "mdc": {
    "error_message": "OK",
    "path": "/structurizr/svg",
    "method": "POST",
    "action": "error",
    "error_code": "500",
    "failure_class_name": "java.lang.IllegalArgumentException",
    "user_agent": "curl/8.0.1"
  },
  "logger": "io.kroki.server.error.ErrorHandler",
  "message": "An error occurred",
  "context": "default",
  "exception": "
java.lang.IllegalArgumentException: An element style for the tag "Software System" already exists.
at com.structurizr.view.Styles.add(Styles.java:33)
at io.kroki.server.service.Structurizr.applyTheme(Structurizr.java:155)
at io.kroki.server.service.Structurizr.convert(Structurizr.java:113)
at io.kroki.server.service.Structurizr.convert(Structurizr.java:149)
at io.kroki.server.service.Structurizr.lambda$convert$0(Structurizr.java:80)
at io.vertx.core.impl.ContextBase.lambda$null$0(ContextBase.java:137)
at io.vertx.core.impl.ContextInternal.dispatch(ContextInternal.java:264)
at io.vertx.core.impl.ContextBase.lambda$executeBlocking$1(ContextBase.java:135)
at io.vertx.core.impl.TaskQueue.run(TaskQueue.java:76)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
at java.base/java.lang.Thread.run(Unknown Source)"
}

Stacktrace reformatted for better readability.

Error does not appear when either removing the theme default or the styles stanza.

Works a expected when loading theme via HTTP. Default theme URL as a workaround: https://raw.githubusercontent.com/structurizr/themes/2ee01b0dd917a77e6427f8cf870e976635fb07f9/default/theme.json

Probably related to #1543.

@bdabelow bdabelow changed the title Cannot override styles when using default theme Structurizr: Cannot override styles when using default theme Jun 26, 2023
@ggrossetie ggrossetie added the 🐞 bug Something isn't working label Jun 26, 2023
@ggrossetie
Copy link
Member

@simonbrowndotje Hey Simon! I hope you are doing fine. What is the correct behavior here? I guess the style defined explicitly should have higher precedence and "override" the theme style? Is that correct? So should I catch IllegalArgumentException and ignore or check that "styles" class does not already have a style element define? Thanks!

@simonbrowndotje
Copy link

See https://structurizr.com/help/themes for more, but essentially any styles defined in the workspace will override any styles defined in the theme ... at least that's how I've implemented it. For example:

workspace {

    model {
        softwareSystem "Software System"
    }

    views {
        theme default

        systemLandscape "SystemLandscape" {
            include *
            autoLayout
        }

        styles {
            element "Software System" {
                background orange
            }
        }
    }
    
}

Generates this diagram:

image

The DSL parser checks for the existence of a style rather than catching IllegalArgumentException, and creates a new style if needed -> https://github.com/structurizr/dsl/blob/master/src/main/java/com/structurizr/dsl/ElementStyleParser.java#L30

@ggrossetie
Copy link
Member

It makes sense, thanks for your reply! I will implement it that way 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants