Do not sort list items alphabetically

[AlexanderSkrock]

This pull request aims to fix #311 in properties-panel together #353 in properties-panel. Currently, it is being developed.

• Remove "shouldSort" property where ever the flag was set to false explicitly
• Implement sorting where necessary
  => Potentially every component which did not explicitly set shouldSort to false, but we should use this opportunity to evaluate where it actually should be sorted.
• Write a note in CHANGELOG.md

Regarding the shouldSort flag, I saw the following throughout this project.

Camunda 7 - Camunda Platform
Usages without the shouldSort flags, thus using the previous default of sorting:

• FieldInjectionGroup
• InMappingGroup
• OutMappingGroup
• ExecutionListenerGroup
• TaskListenerGroup
• ConnectorInputGroup
• ConnectorOutputGroup
• ExtensionPropertiesGroup

Usages with the shouldSort flag set to false, thus already disabling sorting:

• ErrorsGroup
• FormDataGroup
• InputGroup
• OutputGroup
• ProcessVariablesGroup
• ExtensionPropertiesGroup

Camunda 8 - Zeebee
Usages with the shouldSort flag set to false, thus already disabling sorting:

• ExtensionPropertiesGroup
• HeaderGroup
• InputGroup
• OutputGroup

---

Related to bpmn-io/properties-panel#311

[AlexanderSkrock]

Hi @nikku ,
it would be great if you could have a look as well regarding the question which components should keep alphabetical sorting. Thank you in advance.

[nikku]

@AlexanderSkrock I can confirm that non of the fields you mentioned should be sorting.

We can argue about "pure visual only", such as C7 process variables; but then again we can sort on the business logic level.

[AlexanderSkrock]

Seems like my changes from properties-panel were not used. I did one more little fix there for the change dot appearing too late, but I can not confirm the remaining failures locally.

[nikku]

Auto-focus seems to need adjustment in different places (I sketched the change on one example on top of this PR); List#compareFn could be dropped (I pushed that change on top of this PR).

Cf. bpmn-io/properties-panel#353 (comment) for details.

[AlexanderSkrock]

Thank you for having a look! I tried to follow up on your changes regarding auto-focus, but for the ListProps it seems non-trivial as there is no data-entry-id yet, I could hook into.

I wanted to add autoFocusEntry with [data-entry-id="${id}-listItem-${items.length - 1}"] input because that is how its done for all other components, but then I'd need to set the data-entry-id on a very high level, e. g. here.

Otherwise if a data-entry-id with a value-suffix would be okay such as "${id}-listItem-${items.length - 1}-value", then we could incorporate this in the SimpleEntry component in properties-panel core project, e. g. here. Btw, I wondered why we add a prefix to the id there, which we don't for all other entries.

In addition to both of these approaches with the current SimpleEntry component, I could imagine to simply switch to TextField for entries of ListProps, which should work out of the box in combination with autoFocusEntry property set to [data-entry-id="${id}-listItem-${items.length - 1}"] input. (I did not try this one out, yet.)

Would be great, if you could have a look here again, @nikku , as I am not sure which is the best approach. While the last one mentioned seems best to me at first glance, it might have non-obvious changes I am not aware of.

[nikku]

@AlexanderSkrock I'll only be able to follow-up on your comments next week.

[nikku]

@AlexanderSkrock I was able to review your changes, great work! The only thing missing is the auto focus which is now broken in some cases; I fixed it with some follow-up commits.

Was not fully able to grasp some of your thoughts in #1047 (comment). I'd propose to make auto-focus work in the most simple way 🙃. This implies:

• Use the data-entry-id hack for nested lists that return a specific component (a3e3bf4)
• Use simple entry addressing id + '-sub-entry-key' for flat lists (db441b8, dc09c9e)

Of course we could refactor further; but let's take things one step at a time.

[nikku]

Two things missing from my end to consider to get this merged:

• Ensure that properties edited are always in view (cf. screen capture below)
• Rebase on current main branch to resolve conflicts

---

[nikku]

To be clear: I regard data-entry-** auto-focussing as a hack, in a perfect world we'd refactor the list entry to allow passing a dedicated sub-entry property (just a suffix), or provide other resolutions to the common "simply define what to auto-focus" problem:

• We could define an implicit auto-focus, without any explicit configuration - entries can be tagged, and upon creation / "focus" the first entry that is tagged as "could have auto-focus" receives it.
• We could rework the simple and list entries to be able to pass focus in another way.

But then again, looking at the small amount of changes needed here I'm fine to accept the contribution as is.

[nikku]

I'll look into rebasing this. Thanks for your work on this issue 👏

Outdated.

[nikku]

Updated to build on top of @bpmn-io/[email protected].

[nikku]

I can reproduce the test single failure locally and am investigating.

[nikku]

Addressed everything mentioned in #1047 (comment). Waiting for CI to pass.

[nikku]

Thanks for your contribution @AlexanderSkrock. We really apprechiate it 🏅 👏