Fix shape-specific grid container sizing #1294
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alixander
requested changes
May 4, 2023
gavin-ts
force-pushed
the
fix-grid-container-sizing
branch
from
9f9b464
to
5a3fa6b
Compare
alixander
requested changes
May 5, 2023
alixander
approved these changes
May 5, 2023
ejulio-ts
reviewed
May 6, 2023
| shapeType := d2target.DSL_SHAPE_TO_SHAPE_TYPE[dslShape] | ||
| s := shape.NewShape(shapeType, geo.NewBox(geo.NewPoint(0, 0), contentWidth, contentHeight)) | ||
|
|
||
| var fitWidth, fitHeight float64 |
There was a problem hiding this comment.
This should be a method in shape to avoid having to consider this everywhere
There was a problem hiding this comment.
this is because we don't place labels inside the person shape, if we were fitting the shape to the label size, it would use the func below
| } else { | ||
| fitWidth, fitHeight = s.GetDimensionsToFit(contentWidth, contentHeight, paddingX, paddingY) | ||
| } | ||
| obj.Width = math.Max(float64(desiredWidth), fitWidth) |
There was a problem hiding this comment.
This is not obeying the set size. Only if it is smaller than the content.
From what I remember, we simply ignore the set size for containers. Maybe, it should be same here. It doesn't make much sense to be different from the current behavior
There was a problem hiding this comment.
updated tala to use desiredWidth as long as content fits
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Use shape specific sizing for grid containers.
Details
before/after