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

Template selector is misplaced #4617

Open
nikku opened this issue Oct 17, 2024 · 13 comments · Fixed by bpmn-io/bpmn-js-element-templates#130
Open

Template selector is misplaced #4617

nikku opened this issue Oct 17, 2024 · 13 comments · Fixed by bpmn-io/bpmn-js-element-templates#130
Assignees
Labels
bug Something isn't working element templates fixed upstream Requires integration of upstream change good first issue Good for newcomers properties panel spring cleaning Could be cleaned up one day
Milestone

Comments

@nikku
Copy link
Member

nikku commented Oct 17, 2024

Describe the bug

The template selection is misplaced (very low in the properties panel), so that it can easily be overlooked:

image

Steps to reproduce

  1. Open any diagram
  2. Model any task
  3. Locate template picker -> very low in the properties panel

Expected behavior

Template picker is prominently displayed in the properties panel, below the "Documentation" section.

Environment

  • OS: Any
  • Camunda Modeler Version: v5.28.0
  • Execution Platform: Camunda 8
  • Installed plug-ins: None

Additional context

No response

@barmac
Copy link
Collaborator

barmac commented Oct 17, 2024

Let's clarify:

Template picker is prominently displayed in the properties panel.

So below documentation like in 5.25:

image

@barmac
Copy link
Collaborator

barmac commented Oct 17, 2024

We still want to display the execution listeners above the template when it's applied, correct?

@barmac
Copy link
Collaborator

barmac commented Oct 17, 2024

Example:
image

@barmac barmac added good first issue Good for newcomers spring cleaning Could be cleaned up one day ready Ready to be worked on labels Oct 17, 2024
@philippfromme philippfromme added the backlog Queued in backlog label Oct 22, 2024 — with bpmn-io-tasks
@philippfromme philippfromme removed ready Ready to be worked on backlog Queued in backlog labels Oct 22, 2024
@nikku nikku added the ready Ready to be worked on label Nov 1, 2024 — with bpmn-io-tasks
@misiekhardcore misiekhardcore added the in progress Currently worked on label Nov 28, 2024 — with bpmn-io-tasks
@misiekhardcore misiekhardcore removed the ready Ready to be worked on label Nov 28, 2024
@misiekhardcore
Copy link
Contributor

I think this needs to be fixed in bpmn-js-properties-panel as well, so the Zeebe_ExecutionListener group is also above the other groups. Otherwise if Zeebe_ExecutionListener is down on the list, and we put Templates below, those two will always be on the bottom

@nikku
Copy link
Member Author

nikku commented Nov 28, 2024

@misiekhardcore Where else could we anchor the element template selector, so we don't have to move Zeebe execution listeners?

@misiekhardcore
Copy link
Contributor

misiekhardcore commented Nov 28, 2024

Looking at @barmac comment above, we want to also move the execution listeners higher, currently the templates are after execution listeners and execution listeners are by themself low on the list

@nikku
Copy link
Member Author

nikku commented Nov 28, 2024

You mean this?

We still want to display the execution listeners above the template when it's applied, correct?

What would be practical reasons to put execution listeners above the template selector? I don't see it.

The way I see it: Template group is the group following the general section, potentially following the documentation section (documentation = general concern). Everything else follows below, in the natural order. Listeners are less often used than template properties themselves, and hence should go late in the list, above extension properties is a reasonable place when no template is selected, below the domain specific template fields when a template is selected.

@misiekhardcore
Copy link
Contributor

sounds reasonable to me, I just tried to follow whats was written in the comments. If you are both fine with just moving the templates group right below the Documentation group, then I will do that

@barmac
Copy link
Collaborator

barmac commented Nov 29, 2024

I moved the template group below execution listeners for a templated element to prevent conflating execution listeners with the template-specific properties. However, if we see the templated element as just one of the elements, then I agree that there should be no change in positioning. So it's OK to position the template group right below the documentation regardless if template is applied or not, and execution listeners group on the bottom.

@barmac
Copy link
Collaborator

barmac commented Nov 29, 2024

We still want to display the execution listeners above the template when it's applied, correct?

So I take back this.

@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Dec 2, 2024
@github-actions github-actions bot added this to the M84 milestone Dec 2, 2024
@nikku nikku reopened this Dec 2, 2024
@nikku
Copy link
Member Author

nikku commented Dec 2, 2024

This issues is not closed by bpmn-io/bpmn-js-element-templates#130, but instead awaits integration of the upstream change to be fixed. I re-opened and attached fixed upstream to it.

@nikku nikku added the fixed upstream Requires integration of upstream change label Dec 2, 2024
@philippfromme
Copy link
Contributor

Ooops, sorry I missed that it included a closes. Indeed it's only related to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working element templates fixed upstream Requires integration of upstream change 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