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

Alignment of the connector images #418

Open
martonmiklos opened this issue Aug 17, 2024 · 3 comments · May be fixed by #420
Open

Alignment of the connector images #418

martonmiklos opened this issue Aug 17, 2024 · 3 comments · May be fixed by #420

Comments

@martonmiklos
Copy link
Contributor

martonmiklos commented Aug 17, 2024

I have some connectors like this:
kép

However when positioned below the connector pin table they waste a lot of vertical space.

If they could be aligned next to the pin table the diagrams generated would be more compact:
358871815-8d0fc994-07ad-4690-99fa-30be693b9cf8_left_aligned

Is there any way to such an alignment?

If no I would be glad to fix this, any recommendations on the syntax is welcome!

@martonmiklos
Copy link
Contributor Author

I managed to patch the code, the results are quite satisfying for me:
kép

I have added an alignment attribute (with possible values 'under', 'above', 'left', 'right') to the images for connectors only.

Questions:
As the image attributes are documented for the cables and connectors together, would it make sense to have it implemented for cables too?

Code lives here in the case if someone want to peek into it:
https://github.com/martonmiklos/WireViz/tree/add_connector_image_alignment

@kvid
Copy link
Collaborator

kvid commented Sep 7, 2024

@martonmiklos - Thank you very much for this feature suggestion, and I encourage you to create a draft PR with your code suggestions. I have tried your code and and have some comments. If you create a draft PR, then I can place such comments directly at the relevant code changes. My preliminary comments:

  • The optional left/right seems very useful in some cases to improve the diagram readability.
  • Maybe below could be an alternative to under?
  • The above option is currently placing the image above the connector title. IMHO, just above the connector pins is better, but please argue against me, or suggest different alternatives.
  • A property named align or alignment could also be misinterpreted as alignment within the cell when the image is smaller than the available cell space. Is a property named e.g. position perhaps better?
  • For connectors with style: simple, it seems any left or right image is not shown.
  • Ideally, I would like to have the similar options for cables, but it can wait until the code changes for connectors has settled and also checked for possible conflicts with other PRs to be merged in soon.

martonmiklos added a commit to martonmiklos/WireViz that referenced this issue Sep 7, 2024
- Rename image alignment to position
- Rename under to below
- Display the above positioned images right above the pins list

Fixes wireviz#418
@martonmiklos
Copy link
Contributor Author

Hi @kvid

Many thanks for your feedback, I almost thought that the project development stalled totally ;)

Draft PR created see: #420

Maybe below could be an alternative to under?

I am not a native English speaker and my English is not particularly good, so I changed the under to below if it makes more sense to you.

The above option is currently placing the image above the connector title. IMHO, just above the connector pins is better, but please argue against me, or suggest different alternatives.

Yes I agree, fixed it.

A property named align or alignment could also be misinterpreted as alignment within the cell when the image is smaller than the available cell space. Is a property named e.g. position perhaps better?

Position is better I think too, changed it.

For connectors with style: simple, it seems any left or right image is not shown.

Uhm yes, I will fix this.

Ideally, I would like to have the similar options for cables, but it can wait until the code changes for connectors has settled and also checked for possible conflicts with other PRs to be merged in soon

Agreed!

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 a pull request may close this issue.

2 participants