fix: fix the insert plain entry index calculation in case when previousIndex or nextIndex is '0'(falsy)

[AlekseyManetov]

Proposed Changes

Related to #48

fix the insert plain entry index calculation in case when previousIndex or nextIndex is '0'(falsy).

I didn't touch the tests which currently only ensure that an element is present in the menu, not its specific position. The current approach relies a lot on Object.entries or Object.keys manipulations, which doesn't guarantee a consistent order. So we would probably break these tests each time we add a new option, which is what we faced here.

Try out:
npx @bpmn-io/sr #48-make-remove-template-as-first-option

Checklist

Ensure you provide everything we need to review your contribution:

• Contribution meets our definition of done
• Pull request establishes context
  • Link to related issue(s), i.e. Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}
  • Brief textual description of the changes
  • Screenshots or short videos showing UI/UX changes
  • Steps to try out, i.e. using the @bpmn-io/sr tool

[Copilot]

Pull request overview

This pull request fixes an issue where the "replace with plain element" option wasn't always displayed as the first option in the replace menu when replacing elements with applied templates. The fix simplifies the implementation by always inserting the option at position 0 instead of trying to position it relative to other menu options.

Changes:

• Removed complex positioning logic that attempted to place the "remove template" option adjacent to related options
• Simplified implementation to always insert the entry at the start of the menu (index 0)
• Updated tests to verify that the entry appears at the first position

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
lib/element-templates/remove-templates/RemoveTemplateReplaceProvider.js	Simplified the getPlainEntry method to always return index 0, removing the complex positioning logic and unused helper function
test/spec/element-templates/replace-menu/RemoveTemplateReplaceProvider.spec.js	Updated tests to verify that the "replace with plain element" option appears at the first position (index 0)
test/fixtures/element-templates.json	Added Transaction Template and AdHoc SubProcess Template fixtures for testing
CHANGELOG.md	Added entry documenting the fix

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[nikku]

I like how this removes code, but I have some clarifying questions.

I guess I'd rather have the order predictable, so "Transaction" in your example is always at the same place, in the list of replace options.

[AlekseyManetov]

Changed PR description and code changes regarding the comment and discussion with @barmac

[jarekdanielak]

fix the insert plain entry index calculation in case when previousIndex or nextIndex is '0'(falsy)

@AlekseyManetov, to help me and other reviewers understand, could you "translate" this code-specific title to a description of a user-facing problem that this PR fixes?

You included steps to try out, so I know how to run it, but I don't know what to test. 😄

[AlekseyManetov]

@AlekseyManetov, to help me and other reviewers understand, could you "translate" this code-specific title to a description of a user-facing problem that this PR fixes?

You included steps to try out, so I know how to run it, but I don't know what to test. 😄

@jarekdanielak I can't find any user-facing problems related to this fix. It is simply a fix for an obviously incorrect condition(it was broken when we receive '0' index) that was found during the investigation of issue #48, which we decided to close as "not planned." Consider this a minor code refactoring.

[jarekdanielak]

@AlekseyManetov if there is a fix, there must be something that is broken, that we can test either with a unit test or manually. We might not have a test for it now, but we should be able to add one.

Perhaps if there is no user-facing problem at the moment, we can create a test case where the RemoveTemplateReplaceProvider is used in a way that triggers the bug?