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

render: outside connection label masking #447

Merged

Conversation

gavin-ts
Copy link
Contributor

@gavin-ts gavin-ts commented Dec 15, 2022

Summary

We only added label masks when the label is placed over the connection, but when TALA places labels above or below connections the labels could still overlap other connections at times.
This fixes the issue by adding transparent label masks for outside labels (to match shape labels) so that even if the label is over part of another connection the mask will still be applied for improved legibility.

Details

fabricated example where the 111111 label is in an outside position with a mask over another connection

Screen Shot 2022-12-15 at 4 14 51 PM

before/after

Screen Shot 2023-06-09 at 4 06 46 PM

@gavin-ts gavin-ts requested a review from a team December 15, 2022 04:01
@gavin-ts gavin-ts marked this pull request as ready for review December 15, 2022 04:01
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
@ejulio-ts
Copy link
Contributor

I'm not sure..
This seems to make the label ambiguous, as it can be from any edge.
Previously, if the label was over an edge I knew it wasn't related to that edge at all.

@gavin-ts
Copy link
Contributor Author

gavin-ts commented Dec 16, 2022

I'm not sure.. This seems to make the label ambiguous, as it can be from any edge. Previously, if the label was over an edge I knew it wasn't related to that edge at all.

If we need to make the association of outside connection labels clearer that sounds like a separate but related issue to work on. This is solving the legibility of the text itself. I don't think we want to use the fact that the connection can be seen through the text as a way to reason that the label does not belong to that particular connection.

Updated the screenshot to show the full example, not just the zoomed in portion and it is clearer which connection the label belongs to.

@ejulio-ts
Copy link
Contributor

I get it and I agree that it is more about positioning then the mask.
Though, in this example it is not that clear that the label is from the left edge and then 111 is masked but 222 isn't, so..

Can you provide some larger examples, with more edges and other overlaps so that we know how it would look like?

@gavin-ts
Copy link
Contributor Author

gavin-ts commented Jun 9, 2023

with #1383 I think this can have the same behavior as outside shape labels:

Screen Shot 2023-06-08 at 7 29 37 PM

@gavin-ts gavin-ts force-pushed the outside-connection-label-masking branch from c020640 to 3ad501e Compare June 9, 2023 23:07
@gavin-ts gavin-ts requested a review from a team June 9, 2023 23:10
@gavin-ts gavin-ts disabled auto-merge June 16, 2023 02:01
@gavin-ts gavin-ts force-pushed the outside-connection-label-masking branch from 3ad501e to ef60dec Compare June 16, 2023 02:01
@gavin-ts gavin-ts requested review from a team and removed request for a team June 16, 2023 02:01
@gavin-ts gavin-ts merged commit fffb08a into terrastruct:master Jun 16, 2023
2 checks passed
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.

render: add connection label mask even for labels that don't go through middle of connection
3 participants