allow icon for sql_table, class, code/md

[alixander]

Before

After

closes #1558

[alixander]

@gavin-ts there is currently a collision between the icon and label in ELK. I believe it's unrelated to my change and that this code has a bug as described. Can you confirm if so or if I'm just missing something.

[gavin-ts]

yeah it looks like elk hasn't been updated for this. previously all icon positions would be placed in an inside position by default so this would only be possible with custom set positions.

not sure why but elk only seems to be adjusting horizontally for icons but not vertically

d2/d2layouts/d2elklayout/layout.go

Lines 1125 to 1131 in 56a549d

	switch position {
	case label.OutsideLeftTop, label.OutsideLeftMiddle, label.OutsideLeftBottom,
	label.OutsideRightTop, label.OutsideRightMiddle, label.OutsideRightBottom:
	width += d2target.MAX_ICON_SIZE + label.PADDING
	default:
	width = go2.Max(width, d2target.MAX_ICON_SIZE+2*label.PADDING)
	}

also noticed elk layout is still doing some old height manipulations but we already look for intersections with outside labels so these are no longer needed

d2/d2layouts/d2elklayout/layout.go

Lines 1184 to 1187 in 56a549d

	// special handling to start/end connections below label
	if obj.HasOutsideBottomLabel() {
	obj.Height -= float64(obj.LabelDimensions.Height) + label.PADDING
	}

d2/d2layouts/d2elklayout/layout.go

Lines 1112 to 1115 in 56a549d

	// special handling
	if obj.HasOutsideBottomLabel() || obj.Icon != nil {
	height += float64(obj.LabelDimensions.Height) + label.PADDING
	}

d2/d2graph/layout.go

Lines 439 to 445 in 56a549d

	if intersections := labelBox.Intersections(startingSegment); len(intersections) > 0 {
	overlapsOutsideLabel = true
	p := intersections[0]
	if len(intersections) > 1 {
	p = findOuterIntersection(labelPosition, intersections)
	}
	// move starting segment to label intersection point

[gavin-ts]

in the elk icon_positions test it appears this is unaccounted for prior to your PR, it also may not be accounted for in dagre if the label were inside

[alixander]

okay, i'll get in followup then

[alixander]

@gavin-ts
are you sure about the "these are no longer needed"? Many tests change when I remove those:

8ce19c5

[alixander]

i reverted that file, there's still changes: 705225e

[gavin-ts]

8ce19c5#r134821504

looks like we do want the || icon part for this

[gavin-ts]

I think we may actually want this adjustment for all outside labels/icons unless we adjust the spacing in another way
8ce19c5#r134822027

to avoid this

[alixander]

is this captured in #1776 or can you make another issue?

[gavin-ts]

yes this is captured in #1776, all margins are now included in the width/height passed to elk, so elk ends up routing the edge distance to shape + margin, instead of routing to shape then cutting at margin.