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

Persons with configurable default sprites #97

Closed
kirchsth opened this issue Dec 14, 2020 · 16 comments
Closed

Persons with configurable default sprites #97

kirchsth opened this issue Dec 14, 2020 · 16 comments
Milestone

Comments

@kirchsth
Copy link
Contributor

In https://crashedmind.github.io/PlantUMLHitchhikersGuide/scale/scale.html I saw a persion which is more C4 conform.
Shouldn't we use that as standard icon?
grafik

Additional I cannot deactivate the Person icon at all. Did I oversee something?
If it is not possible shouldn't we add a layout switch like "SHOW_PERSON_WITH_ICON()"?

Best regards
Helmut

@Crashedmind
Copy link

Crashedmind commented Dec 15, 2020

Hi @kirchsth

C4 uses the person as the shape outline e.g. https://c4model.com/img/c4-overview.png

Shape outlines in Plantuml are based on the deployment diagram shapes - and C4 person shape outline is not available. See https://plantuml.com/deployment-diagram.

To add a C4 person shape outline would require additions to Plantuml source code.

For PlantUML, I usually do e.g. https://crashedmind.github.io/PlantUMLHitchhikersGuide/C4/C4Stdlib.html#context. This is basically a rectangle shape - with person icon(s).

These are attempts at overcoming this but there are limitations

I don't understand your point on deactivation. Can you give example use case?

thanks,
Chris

@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 15, 2020

Hi Chris,
I know but instead of
grafik
I thought somethink like (person should be white not gray)
grafik
(and therefore I add the link to the person defintion).

And my second point was that I cannot deactivate the icon anymore (that it looks like in the old version)
grafik
and therefore a switch should be offered that the icon can be dactivated (or in an extended version: deactived[default?], "user or other name" (like now), "C4" (something like the icon above), "user3d" (the new version of the current person, https://github.com/Crashedmind/PlantUML-opensecurityarchitecture2-icons/blob/master/User/all.puml
grafik
)

Best regards
Helmut

@Crashedmind
Copy link

thanks Helmut,

for #1 we'd need to change e.g. https://github.com/plantuml-stdlib/C4-PlantUML/blob/master/C4_Context.puml as follows:

  1. replace existing person sprite with sprite from https://crashedmind.github.io/PlantUMLHitchhikersGuide/scale/scale.html
  2. change these to white
    !global $PERSON_BG_COLOR = "#08427B"
    !global $EXTERNAL_PERSON_BG_COLOR = "#686868"

#2 A layout option to make the person appear / not appear could be done with a flag using https://plantuml.com/preprocessing and !if.

I personally prefer the ability to add whatever sprite and attributes I want in the plantuml file.

In general, in the context that I use these diagrams, the audience does not know or care about C4 and has no expectations around the C4 person - presence, shape, outline shape, or color.
The end goal is to convey information that is meaningful to the audience - so I am more likely to present the person in a context that is meaningful to them e.g. a mobile user icon, a hacker icon, a developer icon.... e.g. https://crashedmind.github.io/PlantUMLHitchhikersGuide/C4/C4Stdlib.html#context.
This gives me the ability to change presence, sprite, color, size, etc... number of person icons.

Chris

@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 16, 2020

Hi Chris,

#1) I know, but we call the plantuml-stdlib "C4-PlantUML", therefore I think the default settings should be "C4" style related and therefoe the "C4 user icon"

#2) I had a similar discussion already with @adrianvlupu and HIDE_STEREOTYPE(), details see. Therefore I think the library should offer relevant macros, that an normal user doesn't need to deal with plantuml internals.

I personally prefer the ability to add whatever sprite and attributes I want in the plantuml file.

I aggree, these features are still possible,... But the library introduced the "default user icon" and therefore there should be a possiblity to change it via a predefined macro too. At least there should be a macro that it can be deactivated (without any plantuml background).

BR Helmut

@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 18, 2020

@Crashedmind

maybe that it is more clear with a sample:

I want to add macros which defines the active default sprite

PERSON_SPRITE_NONE()
PERSON_SPRITE_ACTIVE()  -- is default???
PERSON_SPRITE_C4()
PERSON_SPRITE_3D()
PERSON_SPRITE(".....")

and based on that the user gets with following sccript (typically the user defines the default icon only once and the include is on top of the file)

PERSON_SPRITE_ACTIVE()
Person(a, "user with default sprite")
Person_Ext(a2, "extern user with default sprite")

PERSON_SPRITE_C4()
Person(b, "user with C4 sprite no description")
Person_Ext(b2, "extern user with C4 sprite no description")

PERSON_SPRITE_3D()
Person(c, "user", "default 3D")
Person_Ext(c2, "extern user", "default 3D")

PERSON_SPRITE_NONE()
Person(d, "C user", "default NONE")
Person_Ext(d2, "extern user", "default NONE")

!define osaPuml https://raw.githubusercontent.com/Crashedmind/PlantUML-opensecurityarchitecture2-icons/master
!include osaPuml/Common.puml
!include osaPuml/User/all.puml

Person(cus, "user", "Custom", "osa_user_green_architect")
Person_Ext(cus2, "extern user", "Custom", "osa_user_green_architect")

something like
grafik

@simonbrowndotje
Copy link

Just FYI, because this comes up a lot, the C4 model does not prescribe any notation. In other words, there is no "C4 model notation", and the person shape used in the example diagrams is just the person shape rendered by the Structurizr tooling. So feel free to use whatever shapes/icons you like. 🙂

@Crashedmind
Copy link

Thanks Simon for your guidance! 🙏 (and for C4 😃 )

@Crashedmind
Copy link

@kirchsth Personally, I like simple and generic - and I think a user would too:
i.e. As a user, I want an intuitive way to change the icon - for a person
i.e. As a user, I want an intuitive way to change the icon - for a system, or an arrow... e.g. add a bank icon for my bank (because changing the person icon gave context to the audience, and it would be great if I could do that for the other boxes...)

I still prefer "<$user>..", which is simple and generic and customisable to my needs.... e.g. https://crashedmind.github.io/PlantUMLHitchhikersGuide/C4/C4Stdlib.html#context
Giving some examples with the library makes it easy for the user to use this.

But if we do choose to build in support...

Rather than use macros e.g. PERSON_SPRITE_, it may be more intuitive to do e.g.
Person(cus, "user", "Custom") 'No icon specified so get the default
Person(cus, "user", "Custom", "osa_user_green_architect") 'icon specified so get this
Person(cus, "user", "Custom", "") 'icon specified as none so get no icon - or "none" or whatever

similar for System() if we added support...

Alternatively, keyword arguments could be considered for future proofing https://crashedmind.github.io/PlantUMLHitchhikersGuide/procedures/procedures.html?highlight=keyword#keyword-arguments

@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 19, 2020

@simonbrowndotje: you are right, but I think your "Structurizr" is the most known user symbol and therefore it should be part of the C4-PlantUML stdlib too

@Crashedmind, @simonbrowndotje: It didn't change the default user sprite to "C4/Structurizr"; I added it only as an option to the existing sprite. The default user is unchanged. I only added an alternative if somebody likes it (I added a 3D user too).

The main starting point of my implementation was that the person sprite cannot be deactivated at all. The changeable default icon was only a simple add on.

@Crashedmind: The main different between the existing Person(...,sprite) and the PERSON_SPRITE... macro is that the macro changes the default layout of all following Person sprites and not a concrete Person. Additional I saw no other option to deactivate the user sprite, therefore I started with the PERSON_SPRITE_NONE() macro.

E.g. All persons should be displayed with sprite "osa_user_green_architect"

your pattern - you have to change all sprites explicit

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

Person(customer, "Personal Banking Customer", "","osa_user_green_architect")

Enterprise_Boundary(c0, "Big Bank plc") {
    Person_Ext(customer_service, "Customer Service Staff", "", "osa_user_green_architect")
    Person_Ext(back_office, "Back Office Staff", "", "osa_user_green_architect", )
}
@enduml

my pattern - I can change the default sprite for all persions only once

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

PERSON_SPRITE("osa_user_green_architect")

Person(customer, "Personal Banking Customer")

Enterprise_Boundary(c0, "Big Bank plc") {
    Person_Ext(customer_service, "Customer Service Staff")
    Person_Ext(back_office, "Back Office Staff")
}
@enduml

and you still can combine both if required

@startuml
!include https://raw.githubusercontent.com/plantuml-stdlib/C4-PlantUML/master/C4_Context.puml
!define osaPuml https://raw.githubusercontent.com/Crashedmind/PlantUML-opensecurityarchitecture2-icons/master
!include osaPuml/Common.puml
!include osaPuml/User/all.puml

HIDE_STEREOTYPE()
PERSON_SPRITE("osa_user_green_architect")

Person(customer, "Personal Banking Customer", "", "person3D")

Enterprise_Boundary(c0, "Big Bank plc") {
    Person_Ext(customer_service, "Customer Service Staff")
    Person_Ext(back_office, "Back Office Staff")
}
@enduml

mixed

@kirchsth
Copy link
Contributor Author

Maybe the title of the issue is in the meantime misleading, it should be something like
"Persons with configurable default sprites"

@kirchsth kirchsth changed the title C4 conform standard person / deactivate person icon at all Persons with configurable default sprites Dec 19, 2020
@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 19, 2020

Maybe it matches better if I simplify/rename the 5 macros to 2 macros and I add only the 3 predefined sprites.
Then it maps better to e.g. HIDE_STEREOTYPE().

- defined the default setting of all persons in the diagram
HIDE_PERSONSPRITE() - shows the persons (typically) without persion sprite
SHOW_PERSONSPRITE(?sprite) - shows the persons (typically) with a sprite like the samples below

- and the 3 predefined sprites are "person" (current), "person3D", "personC4"

show samples:

SHOW_PERSONSPRITE()  - shows with the predefined "current" person sprite === preconfigured
SHOW_PERSONSPRITE("person")  - shows with the predefined "current" person sprite
SHOW_PERSONSPRITE("person3D")  - shows with the predefined "3D person" sprite
SHOW_PERSONSPRITE("personC4")  - shows with the predefined  "C4/Structurizr" person sprite

!define osaPuml https://raw.githubusercontent.com/Crashedmind/PlantUML-opensecurityarchitecture2-icons/master
!include osaPuml/Common.puml
!include osaPuml/User/all.puml
SHOW_PERSONSPRITE("osa_user_green_architect")  - shows with the custom "osa_user_green_architect" sprite

@adrianvlupu
Copy link
Member

Hi, I agree that person sprite should be optional using HIDE_PERSON_SPRITE() or something, but I think that the way to add custom icons to any container/component/person/system should satisfy most use cases.

@startuml
!include https://raw.githubusercontent.com/adrianvlupu/C4-PlantUML/latest/C4_Container.puml
!define FONTAWESOME https://raw.githubusercontent.com/tupadr3/plantuml-icon-font-sprites/master/font-awesome-5
!include FONTAWESOME/baby.puml

Person(timmy, "Little Timmy", "Human child", "baby")
@enduml

C4_Container Diagram Sample - bigbankplc-icons

This seems a bit overcomplicated

PERSON_SPRITE_ACTIVE()
PERSON_SPRITE_C4()
PERSON_SPRITE_3D()
PERSON_SPRITE()

Why not keep just PERSON_SPRITE(imageName) to let the user change the default person sprite everywhere and HIDE_PERSON_SPRITE().

@kirchsth
Copy link
Contributor Author

kirchsth commented Dec 19, 2020

HI, I agree, I think I found a better solution

Did you overseen my comment (new idea) above yours? - or did you an review?
If it is ok, I would update the my PR like this

- add 2 macros
  HIDE_PERSON_SPRITE()
  SHOW_PERSON_SPRITE(?sprite)

- add 2 persons (to the already existing one)
  person3D
  person4C

Details see above

(The additional persons are important for me a) that I need no additional includes, b) I prefer 3D and C4/Structurizr)

kirchsth added a commit to kirchsth/C4-PlantUML that referenced this issue Dec 21, 2020
… sprites (2 - HIDE_PERSON_SPRITE, SHOW_PERSON_SPRITE, person2)
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this issue Dec 21, 2020
@kirchsth
Copy link
Contributor Author

Hi, I updated and finished my PR.

These are my changes:

- add 2 macros
  HIDE_PERSON_SPRITE()
  SHOW_PERSON_SPRITE(?sprite)

- add 1 person (to the already existing one)
  person2 ()

@adrianvlupu, @Crashedmind: I didn't add the "3D" person, but I leave person2 (I found no lib which contains a similar sprite, and it has no [explicit] C4 relation anymore)

Best regards

PS.: https://github.com/kirchsth/C4-PlantUML/blob/extended/LayoutOptions.md contains a preview of my changes

kirchsth added a commit to kirchsth/C4-PlantUML that referenced this issue Feb 11, 2021
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this issue Feb 11, 2021
@kirchsth
Copy link
Contributor Author

@adrianvlupu or @Potherca: can you merge my approved #101 that we can close it?
Thank you and best regards
Helmut

Potherca added a commit that referenced this issue Feb 18, 2021
#97 Context Diagram supports selectable default person sprites
@Potherca
Copy link
Member

Done! 👍

@Potherca Potherca added this to the v2.1.0 milestone Apr 4, 2021
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

No branches or pull requests

5 participants