Skip to content

Add support for lineThickness in AddRelTag #235

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

Closed

Conversation

fredde-fisk
Copy link

New attempt at #218

@kirchsth
Copy link
Member

kirchsth commented Sep 7, 2022

Hi @fredde-fisk,

thank you for the new skinparam based implementation.

Your solution uses the skinparam arrow Thickness xxx (instead of styles). I checked your implementation and it works basically like below (only that it is documented)

I have only 2 questions:
a) Is Thickness a new skinparam (means do we need a version check and do we have to ignore it in older PlantUML versions)?
b) Can you add $lineThickness as last argument of AddRelTag()... and other calls? It would avoid order/meaning changes in the arguments (if a call uses arguments without keywords)

Thank you and best regards
Helmut

@startuml
!include https://raw.githubusercontent.com/plantuml-stdlib/C4-PlantUML/master/C4_Container.puml

' , $lineThickness=3)
AddRelTag("tag1", $lineStyle=DashedLine())
skinparam arrow<<tag1>> {
     Thickness 3
}

' , $lineThickness=8
AddRelTag("tag2", $lineColor="#800000", $lineStyle=DottedLine())
skinparam arrow<<tag2>> {
     Thickness 8
}

Container(a, "A")
Container(b, "B")
Container(c, "C")

Rel(a, b, " ", $tags="tag1")
Rel(b, c, " ", $tags="tag2")
Rel(a, c, " ")

@enduml

I checked the last non working sample too (and it is working with current version, I didn't check it with an older version)

@startuml
skinparam arrow {
    Color red
    FontColor green
    FontSize 15
}

skinparam arrow<<tag1>> {
        FontColor blue
        Thickness 6
}


skinparam arrow<<tag2>> {
        Color #blue;line.dotted

}

skinparam arrow<<tag3>> {
        FontColor gray
}

[A] -> [B] : hello

[A2] -> [B2] <<tag2>> : hello

[A12] -> [B12] <<tag1>><<tag2>> : hello

[A1] -> [B1] <<tag1>> : hello

[A13] -> [B13] <<tag1>><<tag3>> : hello
[A31] -> [B31] <<tag3>><<tag1>> : hello
@enduml

@kirchsth kirchsth mentioned this pull request Oct 22, 2022
7 tasks
@Potherca Potherca added this to the v2.5.0 milestone Oct 23, 2022
@kirchsth kirchsth self-assigned this Oct 23, 2022
Copy link
Member

@kirchsth kirchsth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I commented it ~50 days ago but it was not visible for you.
I hope it is visible now.
Can you change the argument order?

@@ -550,7 +553,7 @@ $elementSkin
!return $tagEntry
!endfunction

!function $tagRelLegendEntry($tagStereo, $textColor, $lineColor, $lineStyle, $legendText, $legendSprite)
!function $tagRelLegendEntry($tagStereo, $textColor, $lineColor, $lineStyle, $lineThickness, $legendText, $legendSprite)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$lineThickness should be last argument in all calls, that existing diagrams have no unexpected side effects if they use unnamed arguments

@kirchsth
Copy link
Member

kirchsth commented Nov 1, 2022

Hi @fredde-fisk,
during my #232 preparation I found a problem if no color is used in combination with $lineThickness.
I reused your branch and fixed it directly in a new PR #254 (and added a new TestRelationsTags.puml). I hope it is ok for you.
BR Helmut

@kirchsth kirchsth closed this Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants