feat: remove background for non-resizable textboxes

[marstamm]

This PR removes the background for non-resizable textboxes. This ensures we do not overlap with other UI elements, such as task icons or the new context pad entries.

Try it out with

npx @bpmn-io/sr bpmn-io/bpmn-js -l bpmn-io/diagram-js-direct-editing#23-remove-background

closes #23
related to https://github.com/camunda/web-modeler/issues/8477

[philippfromme]

Related to https://github.com/camunda/web-modeler/issues/8477

[nikku]

Find attached several things that I tested:

To me the fact that we edit is a little bit hard to track; maybe that we can improve.

[barmac]

To me the fact that we edit is a little bit hard to track; maybe that we can improve

We could consider using an outline when you activate direct editing via keyboard (e.g. via :focus-visible), opposed to double-click action where the cursor appears close to the click area.

[marstamm]

I don't really like to highlight using the direct-editing border, as you can't adjust it to every case we use it. For bpmn-js, we can adjust Border radius to fit, but especially in dmn-js, the shapes are irregular:

[philippfromme]

You could also use canvas.addMarker to add a CSS class to the element when you edit it so you can change the stroke color and things like that. But I'm not a fan of any of those.

[marstamm]

We also don't ensure the element is actually selected when editing in bpmn-js, so we can't use the existing selection outline either

[philippfromme]

@marstamm Did you look into why there is a slight difference between the text while editing and when it's rendered?

[marstamm]

No, I did not look into that yet

[nikku]

A proposal from within the team discussion:

• Make border entirely an application concern:
  • document how to add it
  • but don't add it

This allows us to apply consistent styling (#39 (comment)) as we see fit in user land.

We'd otherwise depend on internals (border style in this library) when adding borders for, i.e. external labels.

---

Release the whole package as a major version.

[marstamm]

• Make border entirely an application concern:
  • document how to add it
  • but don't add it

This is basically what this PR already does. How to add background is documented in the Changelog as well: https://github.com/bpmn-io/diagram-js-direct-editing/blob/main/CHANGELOG.md#300

[nikku]

This is basically what this PR already does.

I don't think so 😸, let me elaborate:

• What this PR does is to remove border and background, for every label that is not resizable.
• For all other labels, if we wanted to restore the old behavior we'd need to:
  • Dig into the code base or
  • Find the relevant change in the changelog
• I ask for the following:
  • either this library controls background and border (consistently) or it moves that to integrators.

We could take the stance that border and background application concerns (theming) and make this library style agnostic. The current route complicates the situation as parts of the styling (resizable) are direct-editing managed, while others are not (non-resizable labels).

Hence a simpler story for 3.0 could be:

• We do not offer any direct editing box visuals (background, border) but you can do XYZ to add it (via a class or direct style passing)

[nikku]

This could be changed to

Styling of the direct editing box (background, border) is out of scope for this library. Inject your styles depending on your use using the following mechanisms:

* customize css classes
* pass via direct editing provider: ...

[marstamm]

I think the library should offer a sensible default that works out of the box. In the case of resizable labels, this does not work without a border, since we have the "resize corner".
If we want to be consistent, maybe disabling the background should be an application concern and removing it for internal labels should be the way to implement it?

[nikku]

If we want to be consistent, maybe disabling the background should be an application concern and removing it for internal labels should be the way to implement it?

This sounds like a reasonable approach for me, too.

[marstamm]

Then let's revert the changes and implement it in bpmn-js then.

Unfortunately, I published 3.0.0 5 minutes before your original comment, so I'll have to re-release 2.1.2 as 3.0.1 since npm unpublish is broken