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

wip: class diagram cardinality display #705

Merged
merged 2 commits into from
Jun 9, 2019

Conversation

Vrixyz
Copy link
Contributor

@Vrixyz Vrixyz commented Jul 29, 2018

Hello,

This Pull Request is intended to start a discussion on how to tackle the problem of cardinality within mermaid.

See #602

@Vrixyz Vrixyz changed the title wip: #602 cardinality display wip: class diagram cardinality display Jul 29, 2018
@coveralls
Copy link

Pull Request Test Coverage Report for Build 685

  • 0 of 32 (0.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 54.011%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/diagrams/class/classRenderer.js 0 32 0.0%
Files with Coverage Reduction New Missed Lines %
src/diagrams/class/classRenderer.js 1 6.32%
Totals Coverage Status
Change from base Build 671: -0.3%
Covered Lines: 2052
Relevant Lines: 3779

💛 - Coveralls

Copy link
Contributor Author

@Vrixyz Vrixyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

various thoughts I had during development

const nextPoint = path.points[1]

let direction = {x: nextPoint.x - p.x, y: nextPoint.y - p.y}
normalize(direction, 10)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That 10 is not an ideal amount, it should be less if direction is heading UP.

I don't know if we have control on font too, it would break if we have.


let direction = {x: nextPoint.x - p.x, y: nextPoint.y - p.y}
normalize(direction, 10)
const offsettedPoint = {x: p.x + direction.x, y: p.y + direction.y}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this direction might not be enough, we might want to offset even more if that end of the relation has a type (aggregation/composition, etc)

Also, we might want to avoid writing text over the edge/path of the relation.

@pvdyck
Copy link

pvdyck commented Sep 6, 2018

Any chance to merge this superb pull request ? ;-)

@Vrixyz
Copy link
Contributor Author

Vrixyz commented Sep 17, 2018

Thanks for the support.

As much as I’d like something like this merged, I insist that there are quite some problems with that first proposition, text is overlapping with existing nodes, so the end result is not clean. If anyone is interested in improving the pull request, or describing in detail the expected behaviour, go for it. I think I won’t be working on that pull request anymore.

@knsv
Copy link
Collaborator

knsv commented Jun 9, 2019

Merging! Thanks!

@knsv knsv merged commit fe913ec into mermaid-js:master Jun 9, 2019
mgenereu pushed a commit to mgenereu/mermaid that referenced this pull request Jun 25, 2022
Bumps [prettier](https://github.com/prettier/prettier) from 2.5.1 to 2.6.0.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@2.5.1...2.6.0)

---
updated-dependencies:
- dependency-name: prettier
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants