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

Sequence Diagram actor man text should have a unique class #5245

Closed
Ronid1 opened this issue Jan 26, 2024 · 1 comment · Fixed by #5272
Closed

Sequence Diagram actor man text should have a unique class #5245

Ronid1 opened this issue Jan 26, 2024 · 1 comment · Fixed by #5272
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect

Comments

@Ronid1
Copy link
Contributor

Ronid1 commented Jan 26, 2024

Description

Currently, the actor man figure has an actor-man class, but its text has an actor class, which is associated with the default actor type (rectangle). This means there is no way to style only the text of the actor figure or only the text of the actor rectangle.

Steps to reproduce

  1. Create a sequence diagram with at least one man figure actor and one rectangle actor. For example:
sequenceDiagram
    actor Alice
    Alice->>John: Hello John, how are you?
    John-->>Alice: Great!
  1. Add a CSS file with styling for text.actor as described in the documentation
  2. The text in both types of actors will be styled. There is no way to style only one type.

Screenshots

No response

Code Sample

No response

Setup

  • Mermaid version: tested on dev
  • Browser and Version: Chrome

Suggested Solutions

First approach:

  1. Update actor man text tag to have actor-man class
  2. Add actor-text class to the text tag in both the rectangle actor and the man actor

This might cause issues with backwards compatibility for users styling with the text.actor tag, but is overall cleaner IMO.

Second approach:

  1. Add actor-box class to the rectangle actor text tag
  2. Add actor-man class to the figure actor text tag

Better in terms of maintaining the open-close principle, but the text element in the man actor having the same class associated specifically with the rectangle actor is a little confusing.

Either way, this will enable styling the text of actors from both types if wanted, but also unique styling for the text in each type of actor

Additional Context

I'd be happy to work on this. Can implement whichever approach is decided to be better

@Ronid1 Ronid1 added Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect labels Jan 26, 2024
@sidharthv96
Copy link
Member

Let's go with the backwards-compatible approach. We strive not to break existing diagrams (even in major version releases).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Triage Needs to be verified, categorized, etc Type: Bug / Error Something isn't working or is incorrect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants