Canvas: New basic elements

[Develer]

What is this feature?

This PR implements new canvas elements like triangles, clouds, and parallelograms.

Which issue(s) does this PR fix?:

Fixes #83270 #74903

Special notes for your reviewer:

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

Screen.Recording.2024-03-18.at.17.57.38.mov

Added new shapes to existing canvas gdev dashboard for testing data links

Added.new.shapes.to.gdev.datalink.test.dashboard.mov

[grafanabot]

❌ Failed to run Playwright plugin e2e tests.

Click here to browse the Playwright report and trace viewer.
For information on how to run Playwright tests locally, refer to the Developer guide.

[nmarrs]

Great job on these @Develer!

Screen.Recording.2024-03-13.at.9.18.38.AM.mov

I think we need to figure out the question of how we handle these elements styling properties (such as background image / color / border)

With rectangle it is easy as the element fills the entire div making it very straightforward. However with ellipse and now these three added elements it is not clear that the styling should apply to the background of the element vs the element itself

IMO these settings should probably apply to the element themselves vs the background. How we address this might be a little tricky and is dependent on the WIP work here refactoring ellipse's approach to styling options

What I'm not sure about is providing ability to modify both element style and background styles and whether or not we need to update moveable's selection rendering (this might be very tricky, and maybe not necessary given draw.io does a box selection approach regardless of the element)

TLDR; I think we really need to figure out the current WIP of ellipse styling refactor, get to a "good enough" resolution for the time being to unblock this work

cc @lukasztyrala - interested in your thoughts here

[baldm0mma]

LGTM!

[lukasztyrala]

With rectangle it is easy as the element fills the entire div making it very straightforward. However with ellipse and now these three added elements it is not clear that the styling should apply to the background of the element vs the element itself

IMO these settings should probably apply to the element themselves vs the background. How we address this might be a little tricky and is dependent on #83714

From user's perspective, it will be expected that:

1. The element's border option controls the border of the shape, not the container.
2. The element's background option controls the color of the element, not the container.
3. The background coloring based on Fields applies to the background of the element, not the container.

This is something we should address in future iterations. Apart from that, LGTM.

[Develer]

From user's perspective, it will be expected that:

1. The element's border option controls the border of the shape, not the container.
2. The element's background option controls the color of the element, not the container.
3. The background coloring based on Fields applies to the background of the element, not the container.

This is something we should address in future iterations. Apart from that, LGTM.

Yes, that's what I already done in one of the last commits. At least ellipse, triangle, cloud, and parallelogram shapes have their own (not a container's) border and background.

[codeincarnate]

Looks pretty good! Did notice an issue with the cloud being cut off with larger border sizes. Also text layout on the triangle can get a little wonky but those can both be left for follow-ups I think 😄

Just an example of what I mean with the cloud:

Also, is the next step to add support for custom SVG elements 🤔 😆

[Develer]

Looks pretty good! Did notice an issue with the cloud being cut off with larger border sizes. Also text layout on the triangle can get a little wonky but those can both be left for follow-ups I think 😄

Just an example of what I mean with the cloud:

Also, is the next step to add support for custom SVG elements 🤔 😆

yes, I'm aware of it, I didn't find a quick solution, so definitely for a follow-up task 🙂

[nmarrs]

I will work on adding the migration code accounting for ellipse options changes and then I think this is in a "good enough" state to be merged, hopefully we can revisit the known bugs / issues as part of bug cleanup for actual G11 release build 😬

[nmarrs]

I tried to address an issue I noticed where transparency background color is displayed as all black / is broken for the svg elements but wasn't able to figure out what was going wrong / the difference between color handling between svg elements / rectangle. I suspect it may be related to:

grafana/public/app/features/canvas/runtime/element.tsx

Line 199 in c7a973e

if (!SVGElements.has(elementType)) {

I created this github issue to follow up on this

[nmarrs]

I'm going to defer to @Develer to merge this / run through and test things to make sure my changes didn't break anything 😬 I think we have a fair bit to come back to and address as part of polishing this up for main release 😅