BKM as literal expression

[barmac]

https://github.com/bpmn-io/dmn-js/assets/28307541/8448ead1-9ff9-467d-af3b-fbf49b2a7950

Closes #826

Test via npx @bpmn-io/sr bpmn-io/dmn-js#826-bkm-as-literal-expression

[barmac]

TODO:

• [x] Rebase on top of main
• [x] Make sure function kind doesn't change its size unnecessarily
• [x] Include parameters in the suggestions
• [ ] ...

[barmac]

Include parameters in the suggestions

Extracted to https://github.com/bpmn-io/dmn-js/issues/853 as it needs to be solved in https://github.com/bpmn-io/dmn-variable-resolver

[barmac]

This should work but doesn't: npx @bpmn-io/sr bpmn-io/dmn-js#826-bkm-as-literal-expression

I will update the demo website.

[barmac]

There was no need to update the website as the one-liner above works just fine.

[lmbateman]

A couple of questions/comments:

• The items above the expressions don't look very editable. I think the pencil-on-hover works, but there isn't one for the BKM name. Would it make sense to add one? (It would add an extra step for keyboard-only users, but I think that's OK.)

• How much do we want to clean up after the user? If I set up the parameters as in the following screenshot and click out of the popup, there's an extra comma in the parameter list, and the extra parameter row persists in the popup. I would have expected the tool to remove the extra row and the comma. But since the popup doesn't have a Submit button, it's easy to click outside the popup by accident, so I can see the logic in keeping it.

  

• Which leads to the next question: Would it make sense to keep the current behavior, and also add a Submit button that does clean-up? The user has to click to close the popup, so it wouldn't be adding any clicks for mouse users, and only one tab for keyboard users.

• I thought the "F" stood for "function" until just now. We probably have room to spell out the function type if that would be helpful.

[barmac]

I thought the "F" stood for "function" until just now. We probably have room to spell out the function type if that would be helpful.

This is coming from the standard:

(https://www.omg.org/spec/DMN/1.3/PDF#page=110)

My assumption is that the first look should be familiar to anyone who knows the specification.

[barmac]

Which leads to the next question: Would it make sense to keep the current behavior, and also add a Submit button that does clean-up? The user has to click to close the popup, so it wouldn't be adding any clicks for mouse users, and only one tab for keyboard users.

How about this? Not sure if cancel/submit makes sense if we close on external click though.

Name should never be empty according to the spec, so I think we should consider adding some validation in this app.

[barmac]

The items above the expressions don't look very editable

Given we add an input-like border around the name, is it sufficient?

[barmac]

Regarding the parameters, right now we commit the action when it is taken. I imagine that with an explicit submit button, we should commit only when the button is clicked. But in that case I'd rather not close the menu with external click.

[lmbateman]

This is coming from the standard:

Ah, OK, I see that it specifies the initial letter only. For users who aren't necessarily familiar with the spec, my first thought was to add a tooltip, but since the pencil icon appears on hover, that might look weird. What happens if I don't change the function type and write the function in a different language than specified? It would be nice if we can alert users in that case, but I'm guessing it might be difficult to detect the language based on short expressions.

[lmbateman]

Given we add an input-like border around the name, is it sufficient?

It's sufficient for keyboard users, but mouse users might not realize the name is editable. They can edit it in the DRD, though, so the lack of findability might not be a real issue. The more concerning case (as you point out) is when the BKM doesn't have a name. In that case, could we open the BKM with the input-like border visible, perhaps with some text prompting the user to name the BKM? I agree that validation would be helpful, and I think we should add it, but I also like to tell the user what they need to do before telling them they didn't do it. :)

[lmbateman]

Regarding the parameters, right now we commit the action when it is taken. I imagine that with an explicit submit button, we should commit only when the button is clicked. But in that case I'd rather not close the menu with external click.

Agreed.

[barmac]

This is coming from the standard:

Ah, OK, I see that it specifies the initial letter only. For users who aren't necessarily familiar with the spec, my first thought was to add a tooltip, but since the pencil icon appears on hover, that might look weird. What happens if I don't change the function type and write the function in a different language than specified? It would be nice if we can alert users in that case, but I'm guessing it might be difficult to detect the language based on short expressions.

This part is actually unfinished. In the case of non-FEEL languages, one has to use a context with strictly specified keys as described at https://www.omg.org/spec/DMN/1.3/PDF#page=132

Still, I'd consider this to be a follow-up. We don't support context yet, and I'm not even sure if the engine supports non-FEEL functions at all.

As an intermediate step, we could just not show FEEL editor but a text area for non-FEEL languages. This is only about when the non-FEEL language was specified explicitly (e.g. kind set to Java).

[barmac]

Given we add an input-like border around the name, is it sufficient?

It's sufficient for keyboard users, but mouse users might not realize the name is editable. They can edit it in the DRD, though, so the lack of findability might not be a real issue. The more concerning case (as you point out) is when the BKM doesn't have a name. In that case, could we open the BKM with the input-like border visible, perhaps with some text prompting the user to name the BKM? I agree that validation would be helpful, and I think we should add it, but I also like to tell the user what they need to do before telling them they didn't do it. :)

I'll add a border, but I think the validation is a broader topic that we could consider outside of this PR. We don't validate the name anywhere yet. I will create an issue for this.

[barmac]

For validation, I created https://github.com/bpmn-io/dmn-js/issues/856

[barmac]

This is coming from the standard:

Ah, OK, I see that it specifies the initial letter only. For users who aren't necessarily familiar with the spec, my first thought was to add a tooltip, but since the pencil icon appears on hover, that might look weird. What happens if I don't change the function type and write the function in a different language than specified? It would be nice if we can alert users in that case, but I'm guessing it might be difficult to detect the language based on short expressions.

This part is actually unfinished. In the case of non-FEEL languages, one has to use a context with strictly specified keys as described at https://www.omg.org/spec/DMN/1.3/PDF#page=132

Still, I'd consider this to be a follow-up. We don't support context yet, and I'm not even sure if the engine supports non-FEEL functions at all.

As an intermediate step, we could just not show FEEL editor but a text area for non-FEEL languages. This is only about when the non-FEEL language was specified explicitly (e.g. kind set to Java).

I created https://github.com/bpmn-io/dmn-js/issues/857 for the spec-compliant scenario.

[barmac]

Given we add an input-like border around the name, is it sufficient?

It's sufficient for keyboard users, but mouse users might not realize the name is editable. They can edit it in the DRD, though, so the lack of findability might not be a real issue. The more concerning case (as you point out) is when the BKM doesn't have a name. In that case, could we open the BKM with the input-like border visible, perhaps with some text prompting the user to name the BKM? I agree that validation would be helpful, and I think we should add it, but I also like to tell the user what they need to do before telling them they didn't do it. :)

I'll add a border, but I think the validation is a broader topic that we could consider outside of this PR. We don't validate the name anywhere yet. I will create an issue for this.

I added the border and changed the input size to 24 characters (it looked odd with width: 100%;).

[barmac]

Proposed solution for unnamed parameters:

[barmac]

I added a11y tests for basic views and fixed reported issues.

[barmac]

Not sure why the CI is failing right now because it works on my machine 🤡 There must be some problem with npm as it fails on Nx installation while the package is available :/

[lmbateman]

Given we add an input-like border around the name, is it sufficient?

I'm sorry, I misunderstood. I meant that I thought it was enough to have the border around the name when it's in edit mode, since the user can edit the BKM name elsewhere. I'm not sure about the border being visible all the time; without a visible label, the box feels a bit non-standard to me. If we could make the name consistent with the other header items and show the edit button on hover, I think that might be best. @crobbins215, any thoughts? (Apologies again for the misunderstanding.)

[barmac]

I removed the border around the element name.

[barmac]

It looks like we are suffering from https://github.com/npm/cli/issues/4828 on the CI

[barmac]

Given we add an input-like border around the name, is it sufficient?

I'm sorry, I misunderstood. I meant that I thought it was enough to have the border around the name when it's in edit mode, since the user can edit the BKM name elsewhere. I'm not sure about the border being visible all the time; without a visible label, the box feels a bit non-standard to me. If we could make the name consistent with the other header items and show the edit button on hover, I think that might be best. @crobbins215, any thoughts? (Apologies again for the misunderstanding.)

I'd say let's keep this in mind for the follow-up improvements, so that we can learn from user's feedback.

[barmac]

@philippfromme @nikku @marstamm Please have a look at this. There are some follow-up issues linked in the product hub, but I'd like to work on them separately from this PR.

[barmac]

TODO: Make inputs commit on blur, and have a separate command stack (browser native) instead of diagram-js command stack.

[barmac]

TODO: Make inputs commit on blur, and have a separate command stack (browser native) instead of diagram-js command stack.

I will extract this in a separate issue. This is broken already in dmn-js-literal-expression, and it's a bigger issue than I thought because we need to change multiple components to make it work correctly.

[barmac]

I created https://github.com/bpmn-io/dmn-js/issues/859 as a follow-up.