New FX FEEL toggle is misaligned and colors do not match in Web Modeler BPMN properties panel

[christian-konrad]

Describe the Bug

New FX FEEL toggle (#245) is misaligned and colors do not match in Web Modeler BPMN properties panel.

The fx is not in line with the label and is too close to it.

The active state is almost undiscerneable from the disabled one.

Steps to Reproduce

See form editor properties panel from the above PR:

See Web Modeler:

Expected Behavior

• FX to be in line with the label.
• More spacing between label and FX.
• Active state to be clearly visible.

Just as in this picture from the initial PR:

If it is actually a Web Modeler issue, please open a bug report there asap.

Environment

• Host (Browser/Node version), if applicable: [e.g. MS Edge 18, Chrome 69, Node 10 LTS]
• OS: [e.g. Windows 7]
• Library version: [e.g. 2.0.0]

[pinussilvestrus]

@marcellobarile @vpellegrino @bpmn-io/modeling-dev could it be that the web modeler is picking up the correct SVG but the wrong styles from camunda-bpmn-js?

I see that bpmn-js-properties-panel is not updated with [email protected] yet, but the Web Modeler does so. So it's using the updated FX toggle, but with the old styles.

In form-js, it looks correct as @christian-konrad already mentioned ([email protected] is used here): https://camunda-form-playground.netlify.app/

[barmac]

Yeah, that could be the reason as Web Modeler uses camunda-bpmn-js stylesheets which are bundled at release time.

[pinussilvestrus]

I think Christian mentioned that this one should have a higher priority as it's already in the Web Modeler. Can you already give a timeline for this?

As far as I can see, it'd need a properties-panel version bump in bpmn-js-properties-panel, right?

[barmac]

Let's bump it everywhere then. I believe we should also remove v1^ from bpmn-js-properties-panel deps.

[barmac]

OK so if Web Modeler updates to [email protected] and [email protected], the issue should be gone.

[vpellegrino]

@barmac, about camunda-bpmn-js we already updated it in a separate PR.
I just created https://github.com/camunda/web-modeler/pull/4888 to upgrade also bpmn-js-properties-panel. Would you mind having a look and double-check that this issue has gone, when it's possible? Thank you!

[christian-konrad]

@vpellegrino @barmac @pinussilvestrus
it's still broken in Web Modeler, now with another style:

Please make sure to fix it asap

[nikku]

I can confirm that this is the current state, with all dependencies updated (tried with camunda-bpmn-js).

We'd want to root cause where this discrepancy is coming from; why different styles are applied across form-js (individually styled there?) and other libraries that also build on top of @bpmn-io/properties-panel.

[pinussilvestrus]

I think what happens here is that the Web Modeler includes two versions of the properties panel styles in the BPMN page. One from camunda-bpmn-js and one from form-js, which is bundling the properties panel styles together. In case we have a version mismatch, this can collide (cf. what we discussed over in https://github.com/camunda/web-modeler/pull/4888#issuecomment-1575622906).

I think we need to a bit more careful here. Either

• ensure we always include properties panel styles once, no matter what
  • one way to mitigate this is to not use the bundled version of the form-js styles, which we didn't follow up on yet, unfortunately
  • there is a form-js-editor-base.css export that can be used without all extra dependency styles, but we do not export all these extra styles so far (I will create an issue for that)
  • also this one could cause problems, e.g. if we unintentionally use a properties panel stylesheet with not-yet updated form-js, or vice versa
• in case we need different versions, prefix the properties panel styles to a specific context (e.g. forms)
• something else?

Update: I created a follow up issue to consider: bpmn-io/form-js#677

[nikku]

@pinussilvestrus I can reproduce this with stock camunda-bpmn-js, no form-js integration insight. Based on that I assume we're not bundling the right styles with @bpmn-io/properties-panel?

[pinussilvestrus]

@pinussilvestrus I can reproduce this with stock camunda-bpmn-js, no form-js integration insight. Based on that I assume we're not bundling the right styles with @bpmn-io/properties-panel?

Ah, so you mean inside camunda-bpmn-js directly? When I check it out main locally and execute npm run start:cloud, it looks right to me (given the styles that were required).

[nikku]

@pinussilvestrus Thanks for confirming; if this is the look we're going to ship, then it is fine.

Looking at @christian-konrad's comment (#249 (comment)) and also checking it out myself I thought we maybe did not provide the proper spacing for icons.

Here is an alternative version that is slightly more spacious. I find it easier for the eye:

[pinussilvestrus]

@nikku thanks 👍 We agreed that the current state is the version we are going to ship next, but we will follow up with some styling updates (cf. https://github.com/camunda/product-hub/issues/918#issuecomment-1577386470). @RomanKostka will create updated mockups and an issue in this project. I'm happy to take it over then after my FTO 🙂 I will open up the review via demo again then as well and make sure all team members can provide their feedback 👍

Here is an alternative version that is slightly more spacious. I find it easier for the eye:

I agree 👍 /cc @RomanKostka

[nikku]

Moving this issue to backlog.

[RomanKostka]

@pinussilvestrus
Please find the updated styling here:

size and colors (states & background ) has been updated to fit into the existing layout.

[nikku]

@RomanKostka My proposal was to slightly reduce the visual presence of the fx toggle (two px smaller), to also make more space for "background boxes". Did you consider this?

In real world scenarios this does have quite an impact (noise vs. benefit). Also it allows us to render the background box with sufficient spacing.

[nikku]

The decreased height of the fx toggle box works wonders already (colored the background for contrast). So 👍 to go ahead with that simple improvement.

[RomanKostka]

RomanKostka #249 (comment) was to slightly reduce the visual presence of the fx toggle (two px smaller), to also make more space for "background boxes". Did you consider this?

@nikku - Yes, I considered your suggestion with a smaller fx toggle.
In the end, I found that just shrinking the box and removing the background works quite well to get a reasonable size of the "fx". This keeps it visually balanced.

The decreased height of the fx toggle box works wonders already (colored the background for contrast). So 👍 to go ahead with that simple improvement.

👍

[pinussilvestrus]

Please find here the PR with the updated styles: #251

@bpmn-io/modeling-dev is there any fast way to start up a demo of camunda-bpmn-js with a properties panel branch?

[barmac]

For a single demo, I'd suggest to npm link path-to-properties-panel and copy over the CSS from node_modules/@bpmn-io/properties-panel to node_modules/bpmn-js-properties-panel/dist/assets.

[pinussilvestrus]

I rather meant a ready-to-use demo for @RomanKostka and @christian-konrad as we have for form-js (e.g. https://demo-update-fx-styles--camunda-form-playground.netlify.app/). I guess a code sandbox would be the only way or is there something similar for bpmn-js?

[nikku]

@pinussilvestrus Should be possible to build something like that. Just a matter of priority at this point.