Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Camunda Modeler 5.16 Task Inputparmeter Type List alphabetic sorted #3978

Closed
detmar-krause opened this issue Nov 9, 2023 · 10 comments · Fixed by #4324
Closed

Camunda Modeler 5.16 Task Inputparmeter Type List alphabetic sorted #3978

detmar-krause opened this issue Nov 9, 2023 · 10 comments · Fixed by #4324
Assignees
Labels
bug Something isn't working Camunda 7 Flags an issue as related to Camunda Automation Platform 7 good first issue Good for newcomers properties panel spring cleaning Could be cleaned up one day
Milestone

Comments

@detmar-krause
Copy link

detmar-krause commented Nov 9, 2023

Describe the bug

When creating a service task and adding a Inputs parameter as List

nonSorted jpg

and switch to an other task and switch back and open parameter the list ist sorted alphabatically

sorted

also the bpmn xml

             <camunda:inputParameter name="exec_parameters">
                <camunda:list>
                  <camunda:value>SARImageTransformer -joborder ${JSON(imageTransformerContainerJoborderBindFolder).prop("joborderBindFolder").value()}/${imageTransformerJoborderFileName}</camunda:value>
                  <camunda:value>-c</camunda:value>
                  <camunda:value>bash</camunda:value>
                </camunda:list>
              </camunda:inputParameter>

Steps to reproduce

  1. add parameter to Inputs as List
  2. add List elements
  3. leave task
  4. open properties panel of task again
  5. open parameter and expand List elements
  6. Elements are sorted

Expected behavior

Parameter elements of a List should never be sorted or changed in order.

Environment

  • OS: Ubuntu
  • Camunda Modeler Version: 5.16 + 5.17
  • Execution Platform:Camunda 7

Additional context

No response


Depends on bpmn-io/bpmn-js-properties-panel#1047

@detmar-krause detmar-krause added the bug Something isn't working label Nov 9, 2023
@nikku
Copy link
Member

nikku commented Nov 9, 2023

Thanks for opening this bug report. Order shall always follow engine (+ XML) defined order.

@nikku
Copy link
Member

nikku commented Nov 9, 2023

This may be addressed at some point via this (stalled) PR: bpmn-io/bpmn-js-properties-panel#875.

@nikku nikku added backlog Queued in backlog properties panel good first issue Good for newcomers spring cleaning Could be cleaned up one day labels Nov 9, 2023
@nikku nikku added ready Ready to be worked on and removed backlog Queued in backlog labels Nov 10, 2023
@smbea
Copy link
Contributor

smbea commented Nov 14, 2023

I'm assigning to you @nikku since you already had a look. Feel free to un-assign if you can't look into it

@nikku
Copy link
Member

nikku commented Nov 29, 2023

This seems to be a little bigger in complexity, as lists are special (not handled using the default sorting mechanism.

We'll go ahead and remove automatic sorting all together from the properties panel, as discussed in bpmn-io/properties-panel#311 and linked conversation.

@nikku nikku removed their assignment Feb 23, 2024
@nikku
Copy link
Member

nikku commented Feb 23, 2024

Moving this issue back to backlog as we did not find the time to wrap it up.

@nikku nikku added backlog Queued in backlog and removed ready Ready to be worked on labels Feb 23, 2024
@AlexanderSkrock
Copy link
Contributor

Hi there,
I would like to ask about the current state of this issue. Also I had a look into the referenced draft pull request, but I was not able to find out what's missing.

@nikku , if you could potentially spare some time to wrap it up for me, I would volunteer to further work on this issue.

Kind regards
Alexander Skrock

@nikku
Copy link
Member

nikku commented Apr 25, 2024

@AlexanderSkrock we decided to remove alphabetic sorting all together. It does more harm than good and we regret that past decision today 🙈.

There exists a draft PR (bpmn-io/bpmn-js-properties-panel#875) which is stuck, because the topic caused a number of nasty side-effects on other features of the properties panel (such as focussing entries). What we want to do is to remove alphabetic sorting completely from the core (bpmn-io/properties-panel#311).

If you want to take a look into that and assess if it is something you'd want to contribute, please feel free to do so.

@AlexanderSkrock
Copy link
Contributor

AlexanderSkrock commented Apr 25, 2024

So, just to verify that I get you right, @nikku , should we set shouldSort to false by default as mention in your linked issue or did you mean to completely remove the option to use sorting?

Either way, I would give it a try to implement and then analyze occuring focus issues , when I have some spare time.

@nikku
Copy link
Member

nikku commented Apr 26, 2024

We want to remove shouldSort completely:

  • If a consumer uses it print a console.warn that it is no longer supported
  • Automatically all items stop to sort, in all places in the properties panel
  • If sorting is desired, then on the domain level you have to sort your inputs (list of variables, ...)

@nikku nikku added fixed upstream Requires integration of upstream change and removed backlog Queued in backlog labels May 28, 2024
@nikku
Copy link
Member

nikku commented May 28, 2024

This has been implemented via bpmn-io/bpmn-js-properties-panel#1047 and needs to be integrated into the modeler.

To be shipped with v5.24.0 🙂

@nikku nikku added this to the M77 milestone May 28, 2024
@nikku nikku added the Camunda 7 Flags an issue as related to Camunda Automation Platform 7 label May 28, 2024
nikku added a commit that referenced this issue May 29, 2024
nikku added a commit that referenced this issue May 30, 2024
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label May 30, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label May 30, 2024
nikku added a commit that referenced this issue May 30, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Camunda 7 Flags an issue as related to Camunda Automation Platform 7 good first issue Good for newcomers properties panel spring cleaning Could be cleaned up one day
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants