Skip to content

Conversation

@huyenngn
Copy link
Contributor

No description provided.

@huyenngn huyenngn marked this pull request as ready for review August 1, 2024 12:30
Because of elkjs limitations edge labels cannot be displayed properly at this point without sacrificing the desired layout.
Edge labels will be omitted completely as to not create confusion with them showing up on the top left corner.
They should be added back when the issues with the edge labels got fixed.
@huyenngn huyenngn force-pushed the feat-element-relation-view branch from 563d5e5 to 88e15a1 Compare August 2, 2024 08:29
@huyenngn huyenngn requested a review from micha91 August 5, 2024 08:11
Copy link
Collaborator

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

Good job. I am not sure about the naming of this View. We bring ExchangeItems and Classes with ExchangeItemElements into a relation. But we also reuse the ClassTreeView. Maybe ExchangeItemClassTreeView? I am open for ideas.

Never the less, we need documentation here:

  • Add a exchange_item_class_tree_view.md in docs (add it to mkdocs.yml)
    • Hint to the problem of routing edges (ExchangeItemElements) which could be solved when Libavoid comes publically available
  • Describe collection, parameters and give examples (reuse the test case)
  • Add the needed SVGs to the docs/gen_images.py

@huyenngn huyenngn force-pushed the feat-element-relation-view branch 3 times, most recently from 5c4fab9 to 0338310 Compare August 13, 2024 08:32
@huyenngn huyenngn requested a review from ewuerger August 13, 2024 08:34
@huyenngn huyenngn force-pushed the feat-element-relation-view branch from 0338310 to 84ebc9a Compare August 13, 2024 09:29
Copy link
Collaborator

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

Just rename it back to exchange_item_relation_view.
LGTM.

@huyenngn huyenngn requested a review from ewuerger August 21, 2024 10:06
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.

3 participants