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

(timing diagram) message labels ignore default style #1746

Closed
chrisaga opened this issue Apr 19, 2024 · 5 comments
Closed

(timing diagram) message labels ignore default style #1746

chrisaga opened this issue Apr 19, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@chrisaga
Copy link

Describe the bug
Message labels have their own font name, style and size.
They don't follow defaultFontName, defaultFontColor nor defaultFontSize.
The only styling they accept is inheritance of message arrow's color.

To Reproduce
Steps to reproduce the behavior:

  1. Copy one of the classic timing diagram examples with message arrows and render it
  2. See that the message label's font is different from other fonts
  3. Try to change default font style such as:
skinparam defaultFontName "Serif"
skinparam defaultFontSize 18
skinparam defaultFontColor Blue
  1. Some text rendering change in the diagram but message labels don't

Expected behavior
Message labels follow default styling.

Conclusion
This should be a first step toward targeted styling of message labels which was what I was trying to achieve. I realize that would be a feature request.
IMHO not following default style is a bug. Feel free to qualify this as a feature request too if I am wrong.

@The-Lum
Copy link
Collaborator

The-Lum commented Apr 20, 2024

Hi @chrisaga, and all,

Here is a corresponding example:

From:

@startuml

robust "DNS Resolver" as DNS
robust "Web Browser" as WB
concise "Web User" as WU

@0
WU is Idle
WB is Idle
DNS is Idle

@+100
WU -> WB : URL
WU is Waiting
WB is Processing

@+200
WB is Waiting
WB -> DNS@+50 : Resolve URL

@+100
DNS is Processing

@+300
DNS is Idle

@WU
@200 <-> @+150 : {150 ms}
@enduml

To:

@startuml

skinparam defaultFontName "SansSerif"
skinparam defaultFontSize 10
skinparam defaultFontColor green

robust "DNS Resolver" as DNS
robust "Web Browser" as WB
concise "Web User" as WU

@0
WU is Idle
WB is Idle
DNS is Idle

@+100
WU -> WB : URL
WU is Waiting
WB is Processing

@+200
WB is Waiting
WB -> DNS@+50 : Resolve URL

@+100
DNS is Processing

@+300
DNS is Idle

@WU
@200 <-> @+150 : {150 ms}
@enduml

Regards,
Th.

@The-Lum The-Lum removed the triage label Apr 20, 2024
@The-Lum
Copy link
Collaborator

The-Lum commented Apr 20, 2024

Hi @chrisaga, and all,

Thanks for the report,

Yes, it is a bug, because arrow's label seems to take the color from arrow LineColor and not from arrow FontColor...

Here is an example:

@startuml

<style>
arrow {
	FontName SansSerif
	FontSize 11
	FontColor green
	LineColor red
}
</style>

robust "DNS Resolver" as DNS
robust "Web Browser" as WB
concise "Web User" as WU

@0
WU is Idle
WB is Idle
DNS is Idle

@+100
WU -> WB : URL
WU is Waiting
WB is Processing

@+200
WB is Waiting
WB -> DNS@+50 : Resolve URL

@+100
DNS is Processing

@+300
DNS is Idle

@WU
@200 <-> @+150 : {150 ms}
@enduml

Regards,
Th.

@The-Lum The-Lum added the bug Something isn't working label Apr 20, 2024
@jimnelson372
Copy link
Contributor

jimnelson372 commented May 10, 2024

Hello. I have a fix for this, but I have a question about how to deal with the default skins when an element such as the timingdiagram.arrow needs default skin settings. To have the current defaults on the message text, it needs the Font settings added in the skins. Otherwise it will get the default Font settings for the timingdiagram itself, which is the bolded player headings. This, of course, is in the absence of the the scripts overriding the defaults either by skinparam settings, as in the original script provided by @chrisaga for this issue , or by the arrow style settings in the script provided by @The-Lum.

With my current changes, here is what Chris's script generates, given the skinparam settings to make all fonts SanSarif/green/10.

image

And here is what @The-Lum's script generates using the Style/arrow settings to make the arrow line red and the Font SanSarif/green/11 :

image

I've also added 2 extra pixels to the left of the labels, so that they aren't touching the arrows.

Now, if I update my plantuml.skin file settings on the timingDiagram, as follows:

timingDiagram {
...
	arrow {
	    FontName Serif
	    FontSize 14
            FontStyle plain
            FontColor darkblue
            ...
	}
..
}

And go back to the original script above without any overrides from default, I get what looks just like the current diagram, other than the 2 pixel space I just mentioned so that the message isn't right on the line.

image

(And, yes, the hardcoded Font settings for the message was Sarif/plain/14. I am curious why it is larger than labels such as on the timing constraint arrow, but it currently is. This fix would be an opportunity to choose a new default size/style.):

However, if the .skin files are not all updated with this change, then the message now defaults as I mentioned above, to the player headers font settings. Here is what it looks like without my plantuml.skin changes:

image

It's not too bad, but it is different than the expected defaults. I've thought about somehow detecting that the .skin file doesn't provide the Font settings on the timingDiagram.arrow, and then hardcoding the original default if no override settings are in the script, but I'm not sure how to do that, nor if I need to. I'm seeking advice on how new diagram features than require .skin changes have been managed in the past.

Best regards,

Jim Nelson

@arnaudroques
Copy link
Contributor

It's not too bad, but it is different than the expected defaults. I've thought about somehow detecting that the .skin file doesn't provide the Font settings on the timingDiagram.arrow, and then hardcoding the original default if no override settings are in the script, but I'm not sure how to do that, nor if I need to. I'm seeking advice on how new diagram features than require .skin changes have been managed in the past.

Many thanks for your investigation!
Let's keep the code as simple as possible: so do not try to detect if the .skin file doesn't provide the Font settings on the timingDiagram.arrow.
Even if it slightly changes the rendering of previous diagrams, I think this is an acceptable change.
I am not even sure that anyone would notice the change :-)

The more important thing is that the next-to-be behavior is more consistent than the actual behavior.

jimnelson372 added a commit to jimnelson372/plantuml_fork that referenced this issue May 10, 2024
jimnelson372 added a commit to jimnelson372/plantuml_fork that referenced this issue May 10, 2024
arnaudroques added a commit that referenced this issue May 11, 2024
Fixes issue #1746: (timing diagram) message labels ignore default style
@The-Lum
Copy link
Collaborator

The-Lum commented May 26, 2024

Hi all,

[This is an Issue Review] 👀
This is now fixed on V1.2024.5.

Regards.

@The-Lum The-Lum closed this as completed May 26, 2024
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

No branches or pull requests

4 participants