fix: Enable scrolling on FEEL popup editor

[vsgoulart]

The scrollbar in the FEEL popup editor was hidden when the text was too long.

2023-12-15.15-27-38.mp4

Closes #316

[vsgoulart]

@marstamm I drilled as far as I could in the history and from what I've seen you added these styles. I couldn't notice anything this was fixing, but maybe I missed something and you remember why this was necessary

[nikku]

@vsgoulart as @marstamm is out at the moment I suggest to dig for the commit that he introduced this and see if it exposes further context.

Also, please verify this works for the proprties panel core, with long text, too.

[nikku]

Happy to merge if it also works in feel editors embedded inside the properties panel. There we want the editor to always have full height and auto-resize with the content:

[christian-konrad]

@nikku is this style affecting the native property panel fields? Then we should probably find a dedicated fix in the popup editor. The native panel editors should not have a scroll bar and autofit as you said

[nikku]

@nikku is this style affecting the native property panel fields?

I don't know, but I gave this pointer to @vsgoulart to be able to assess the end-to-end impact of removing a prior fix (again).

[Skaiir]

Analysis of the cm-scroller stuff, it's kind of a fascinating evolution that started multiple years ago.

• first version of the feel component shipped with .cm-scroller: overflow-x: none, presumably to hide the thick scrollbar that would occur when working with single line expressions
• when feel textareas joined the party (which are now used everywhere), this was extended to overflow in both directions, presumably for the same reason (as resizing was introduced)

Example of what it looks like on windows with the scrollbars on:

• eventually this was scoped to .bio-properties-panel-feel-editor-container .cm-scroller to stop the properties panel from interfering with our other codemirror instances
• niklas re-used the container classes to bring over as much of the styling of the properties panel proper into the popup, however this means there's also the disabling scroller stuff in there.

Takeaways

So, two ways we can fix this:

• re-enable scrollbars only in the popup, this would mean we'd use bio-properties-panel-popup to scope another css rule
• re-enable scrollbars in both, merge this fix

My opinion:

• scrollbars should be visible in the properties panel (non-popup), but we should use a subtle custom implementation of them (something like https://malte-wessel.com/react-custom-scrollbars/).
• for now, let's just re-enable the scrolling within the popups only
• lets also raise some follow-ups as we see are appropriate

Other observations:

• the positioning of the feel popup editor on open is quite janky, especially when the text content is large
• the popup button prevents resizing of the inner text
• the resizing option might not be serving a purpose anyways, since we autosize
• feel textareas should be height limited, otherwise they become unmanageable
• feel textareas and regular textareas behave differently in general and should be made more similar

[Skaiir]

I've sent through my own suggested fix.

[philippfromme]

Regarding your other observations, did you check whether there are existing issues for them?

[Skaiir]

@philippfromme They don't, at least not directly though some issues are already raised that are similar. For those I just wanted to see what people's thought were but I plan to raise follow-ups

[nikku]

Let's clearly separate styles for popup editor and properties panel (cf. https://github.com/bpmn-io/internal-docs/issues/825).

Let's then ship an isolated fix for the popup editor to make scrolling possible there.

The properties panel is meant to be resized to "fit contents".

[nikku]

What we could consider is to add automatic line breaks (always fit content). This is exactly how text area elements work, too.

[marstamm]

What we could consider is to add automatic line breaks (always fit content). This is exactly how text area elements work, too.

I think this makes sense - we evolved from single-line to multi-line editor and did not change the line-break behavior along with it. As we always have a "Text Area style" editor, we should not use the line-break behavior of normal input fields

[marstamm]

@Skaiir when testing on properties panel, the scroll-bars are interfering with the Close button. This looks broken and not intended:

Are there upstream changes that I am missing which change these styles?

[marstamm]

Cf. #319 (comment)

I tried it out and it is possible with minimum effort. As it is a one-line fix, I added this directly to this branch (506fba7)

With this, we only need Vertical scrolling in both the Views.

@Skaiir can you take a look at #319 (comment) ? Seems like this can be fixed by just moving the scroll bar all the way to the right if we don't need horizontal scrolling

[nikku]

Looks great to me (including text area style line-breaks) :).

[vsgoulart]

@marstamm The scroll padding should be fixed now

[nikku]

This works nicely for me now.

A required additional (and potentially simple) fix is to move the popup editor icon to the top of the input field. You'll otherwise be forced to scroll all the way down for longer FEEL expressions to toggle the popup editor 🙈.