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

enable hyperlinks in C4 elements and fix samples #72

Conversation

SebastienAndreo
Copy link

I implemented the possibility to embed hyperlink in the C4 elements. This feature makes your svg elements clickable and navigable.

I also corrected some incompatible changes in the plantuml engine.

@anorm
Copy link

anorm commented Oct 8, 2020

I usually accomplish the same thing by writing

@startuml
!include <C4/C4_Context>
System(google[[https://google.com]], "Google", "An internet search engine")
@enduml

Your addition is more user friendly, though.

@stale
Copy link

stale bot commented Dec 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Dec 7, 2020
@stale stale bot closed this Dec 14, 2020
@SebastienAndreo
Copy link
Author

CAn we reopen the MR

@Potherca Potherca added not-stale Stop issue from being marked stale by bot and removed wontfix This will not be worked on labels Feb 1, 2021
@Potherca
Copy link
Member

Potherca commented Feb 1, 2021

@SebastienAndreo We're still calibrating the stale bot. At your request, I have re-opened this MR and marked it so the bot won't auto-close it again, so the other maintainers have some more time to take a look at your contribution.

@Potherca Potherca reopened this Feb 1, 2021
@FastNinja
Copy link

what would it take to move this PR forward?

@Potherca
Copy link
Member

@FastNinja What needs to happen is that the commits are rebased (to resolve the code conflicts) and a review needs to take place. Currently 2 reviewers are required so, not counting myself, at least one other maintainer needs to find time for a review.

@adrianvlupu
Copy link
Member

Hi,

Because adding links to entities is simple enough and doesn't require it to pass through the create System/Container/Component function, I think overcomplicating the functions should be avoided.

!unquoted procedure Container($alias, $label, $techn, $descr="", $sprite="", $tags="")

Personally I use this to create links:
Container(example, "Example", "Example", "Example") [[https://google.com]]

@Potherca
Copy link
Member

Potherca commented Mar 14, 2021

Both suggestions (by @anorm and @adrianvlupu) seem easy enough and would not require any code changes:

System(google[[https://google.com]], "Google", "An internet search engine")
/' or '/
Container(example, "Example", "Example", "Example") [[https://google.com]]

@SebastienAndreo As choosing which features to add/implement is just as important as deciding what not to, we might have to consider closing this MR without merging its proposed changes into the codebase...

Either way, we should make sure we add a section to the documentation about how to add links.


As most active contributors/maintainers, can we get your opinions on whether this should be included or not? 👍 / 👎

@SebastienAndreo
Copy link
Author

I tested @adrianvlupu 's solution and it works fine in my context also.
You can close the MR

@kirchsth
Copy link
Contributor

@Potherca, @adrianvlupu, @RicardoNiepel, @aheil: the link pattern works with elements and even Rel() calls can be modelled with links like below:

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

Person(personAlias, "Label", "Optional Description")
Container(containerAlias, "Label", "Technology", "Optional Description")

Rel_R(personAlias, containerAlias, [[http://plantuml.com/class-diagram mixed label]]) 

' this would produce a cleaner model
Rel_R(personAlias, containerAlias, "clean modelled", $link="http://plantuml.com/class-diagram produces")
@enduml

We could add it like the $tags="..." extension: In all my samples I set the $tags as "keyword argument" that it can be used if required and if not required it can be ignored in the calls (without any empty default arguments)

!unquoted procedure System($alias, $label, $descr="", $sprite="", $tags="", $link="")
...
System(example, "Example", $link="https://google.com")

If we want a cleaner model we should add it as "keyword argument".

@aheil
Copy link
Member

aheil commented Mar 18, 2021

Personally, I don't see any reason not to allow clickable hyperlinks.
That would add additional value for documentation.

If I understood this right, it works for SVG export anyway.
How would those links appear in PNGs or other export formats? Would this confuse the user/viewer of the diagram?

Actually, that's something I always wanted to add to my EIP extension as well.

kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Mar 19, 2021
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Mar 19, 2021
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Mar 19, 2021
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Mar 19, 2021
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Mar 19, 2021
@Potherca
Copy link
Member

I am closing this in favor of #135.

@Potherca Potherca closed this Apr 11, 2021
@Potherca Potherca removed the not-stale Stop issue from being marked stale by bot label Apr 11, 2021
Potherca added a commit that referenced this pull request Apr 11, 2021
#119, #72: All elements and relationships can support hyperlinks (based on #126)
kirchsth added a commit to kirchsth/C4-PlantUML that referenced this pull request Apr 28, 2021
…- image with links is added as svg (fix merge in extended)
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.

None yet

7 participants