plantuml-stdlib / C4-PlantUML

C4-PlantUML combines the benefits of PlantUML and the C4 model for providing a simple way of describing and communicate software architectures
MIT License
6.41k stars 1.1k forks source link

System() etc. with sprites, but no label #281

Closed mtnpke closed 1 year ago

mtnpke commented 1 year ago

Hi!

I think a nice feature to have would be to be able to declare systems, containers etc. that only contain a sprite and no label. This is useful for including a logo sprite that already has text in it, so having it as label text would be repetitive. Right now, if you write for example System_Ext(Test, "", $sprite="img:test.png"), you will get an extra blank line in the rectangle after the image, which is visually unpleasing.

Potherca commented 1 year ago

I can understand the issue regarding superfluous whitespace. I can really mess with the harmony of an image.

Current Fixed

A quick dig in the code leads me to believe this shouldn't be too hard to resolve, although I would have to check before committing myself to anything.

As far as I can see, this behaviour comes from the $getComponent, $getContainer, $getPerson and $getSystem, and $getNode functions, respectively.

For getSystem and getPerson it would be neccesary to add an extr if to check for the label. The current code reads:

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Context.puml#L342-L344

That would have to be changed to:

!if ($descr == "") && ($sprite != "")
  !if ($label == "") 
    !return $getSprite($sprite)
  !else
    !return $getSprite($sprite)+'\n== '+$breakLabel($label)
  !endif
!endif

For getComponent, getContainer, and getNode* the code is:

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Component.puml#L65

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Container.puml#L66

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Deployment.puml#L59

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Deployment.puml#L74

https://github.com/plantuml-stdlib/C4-PlantUML/blob/e374b3468a3341df8f909901bef545b075a0a978/C4_Deployment.puml#L89

which would need to be changed to:

  !if ($label != "")
    ...
  !endif

@kirchsth Can I ask you for feedback and/or to tell me what I am missing? 😁

kirchsth commented 1 year ago

@Potherca: Thank you for the implementation.

I think it is complete I only would

!function $getComponent($label, $techn, $descr, $sprite) $getElementBase($label, $techn, $descr, $sprite) !endfunction

!function $getContainer($label, $techn, $descr, $sprite) $getElementBase($label, $techn, $descr, $sprite) !endfunction

!function $getPerson($label, $descr, $sprite) !if ($sprite == "") && ($defaultPersonSprite != "") !$sprite = $defaultPersonSprite !endif $getElementBase(.., $techn="", ...) !endfunction

!function $getSystem($label, $descr, $sprite) $getElementBase(.., $techn="", ...) !endfunction



- and update $getNode(), $getNode_L(), $getNode_R() with a common $getNodeBase() implementation (but with very low priority)

Please be aware that I didn't check anything. 

**And I think we should at least test if all arguments are empty that it is not "crashing".**

Thank you and best regards
Helmut
kirchsth commented 1 year ago

Hi @mtnpke, I implemented it in https://github.com/plantuml-stdlib/C4-PlantUML/pull/285. Can you please check it? Thank you and BR Helmut

mtnpke commented 1 year ago

@kirchsth Yeah, it's working great :) @Potherca Thanks, much obliged!