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.

Sign up for GitHub

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

Jump to bottom

fix(replace-menu): keep existing loop characteristics #2251

Merged
merged 2 commits into from
Oct 31, 2024
Merged

fix(replace-menu): keep existing loop characteristics #2251

merged 2 commits into from
Oct 31, 2024

Conversation

philippfromme
Copy link
Contributor

@philippfromme philippfromme commented Oct 28, 2024 •
edited
Loading

Partially addresses camunda/camunda-modeler#4310.

brave_V1BCetnO9u

Related to camunda/camunda-modeler#4310

Proposed Changes

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Oct 28, 2024
@philippfromme philippfromme force-pushed the fix-loop-characteristics branch from e7b24e6 to c41e232 Compare October 28, 2024 12:51
@philippfromme philippfromme mentioned this pull request Oct 28, 2024
Closed
@nikku
Copy link
Member

nikku commented Oct 30, 2024

@philippfromme Could you elaborate ("text") what part of the linked issue is tackled, and what part is not addressed (yet)?

nikku
nikku reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires test cases to verify this behavior.

nikku
nikku requested changes Oct 30, 2024
View reviewed changes
Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs test case to verify things work.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Oct 30, 2024
@philippfromme
Copy link
Contributor Author

It fixes the immeditate issue for the user when switching between the multi instance loop characteristics. But the underlying issue is fixed by camunda/zeebe-bpmn-moddle#66 which is verified by camunda/camunda-bpmn-js-behaviors#84.

@nikku
Copy link
Member

nikku commented Oct 31, 2024

Is this fix here needed then? I assume yes, because also without zeebe stuff involved, we want to be forgiving (keep standard multi-instance loop properties, if possible)?

@philippfromme
Copy link
Contributor Author

It's not strictly necessary but it avoids an unnecessary replacing of the bpmn:MultiInstanceLoopCharacteristics element.

@nikku
Copy link
Member

nikku commented Oct 31, 2024 •
edited
Loading

Ok, happy to merge if we have another test case for this, verifying a user relevant behavior.

@philippfromme
Copy link
Contributor Author

Ok, will add.

@philippfromme
Copy link
Contributor Author

Added test coverage via bfa184b.

@philippfromme philippfromme requested a review from nikku October 31, 2024 13:30
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Oct 31, 2024
@nikku
Copy link
Member

nikku commented Oct 31, 2024

Great stuff!

@nikku nikku merged commit e937bd9 into develop Oct 31, 2024
12 checks passed
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 31, 2024
@nikku nikku deleted the fix-loop-characteristics branch October 31, 2024 15:05
@nikku
Copy link
Member

nikku commented Oct 31, 2024

As good measure, already add a CHANGELOG entry to indicate this change.

@github-actions github-actions bot mentioned this pull request Nov 6, 2024
Closed
@ElRaptorus ElRaptorus mentioned this pull request Jan 9, 2025
Merged
ElRaptorus added a commit to 5minds/bpmn-js that referenced this pull request Feb 4, 2025
@ElRaptorus
Bump to 18.1.2 (#9)
a1d927b 
# Changes 

Siehe Changelog:
https://github.com/bpmn-io/bpmn-js/blob/develop/CHANGELOG.md

## 18.1.2

* `FIX`: canvas `autoFocus` must explicitly be enabled
([bpmn-io/diagram-js#956](bpmn-io/diagram-js#956))
* `FIX`: properly integrate `zoomscroll` with canvas focus
([bpmn-io/diagram-js#956](bpmn-io/diagram-js#956))
* `FIX`: properly integrate `movecanvas` with canvas focus
([bpmn-io/diagram-js#956](bpmn-io/diagram-js#956))

## 18.1.1

* `FIX`: adjust search to prioritize start of word and exact matches
([bpmn-io/diagram-js#953](bpmn-io/diagram-js#953))
* `FIX`: ignore whitespace when searching
([bpmn-io/diagram-js#954](bpmn-io/diagram-js#954))

## 18.1.0

* `FIX`: clear selection when opening search pad
([bpmn-io/diagram-js#947](bpmn-io/diagram-js#947))
* `FIX`: correct dangling selection after search pad interaction
([bpmn-io/diagram-js#947](bpmn-io/diagram-js#947))
* `DEPS`: update to `diagram-js@15.2.2`

## 18.0.0

* `FEAT`: remove `outline` from `Viewer` modules
([bpmn-io#2135](bpmn-io#2135))
* `FEAT`: make `Canvas` a focusable element
([bpmn-io/diagram-js#662](bpmn-io/diagram-js#662))
* `FEAT`: implicit keyboard binding
([bpmn-io/diagram-js#662](bpmn-io/diagram-js#662))
* `FEAT`: integrate with global `search`
([bpmn-io#2235](bpmn-io#2235))
* `FEAT`: integrate `popup-menu` with `search`
([bpmn-io/diagram-js#932](bpmn-io/diagram-js#932))
* `FEAT`: recognize modern `search` tokens in `search-pad`
([bpmn-io/diagram-js#932](bpmn-io/diagram-js#932))
* `FIX`: correctly handle duplicate entries and whitespace in `search`
([bpmn-io/diagram-js#932](bpmn-io/diagram-js#932))
* `FIX`: find `search` terms across all keys
([bpmn-io/diagram-js#932](bpmn-io/diagram-js#932))
* `FIX`: `search` always returns tokens for matched items
([bpmn-io/diagram-js#932](bpmn-io/diagram-js#932))
* `FIX`: prevent crash during label adjustment
([bpmn-io#2239](bpmn-io#2239))
* `FIX`: keep existing loop characteristics when toggling through the
replace menu ([bpmn-io#2251](bpmn-io#2251))
* `FIX`: prevent covering multi selection with black box in `Viewer`
([bpmn-io#2135](bpmn-io#2135))
* `FIX`: generate types for main entry
([`986e2bb`](bpmn-io@986e2bb))
* `FIX`: correct handling of group name with whitespace only
([bpmn-io#2231](bpmn-io#2231))
* `DEPS`: update to `bpmn-moddle@9`
([bpmn-io#2114](bpmn-io#2114))
* `DEPS`: update to `diagram-js@15.1.0`
* `DEPS`: update to `diagram-js-direct-editing@3.2.0`

### Breaking Changes

* Require `Node >= 20`
* `Canvas` is now a focusable element and provides better support for
native browser behaviors. Focus can be controlled with new `focus` and
`restoreFocus` APIs
([bpmn-io/diagram-js#662](bpmn-io/diagram-js#662)).
* Keyboard is now implicitly bound to canvas SVG element. Calls to
`keyboard.bind` and `keyboard.bindTo` now result with a descriptive
console error and have no effect
([bpmn-io/diagram-js#662](bpmn-io/diagram-js#662)).

## 17.11.1

* `FIX`: handle searching elements without labels
([bpmn-io#2232](bpmn-io#2232),
[bpmn-io#2234](bpmn-io#2234))

## 17.11.0

* `FEAT`: align search styles with other popups
([bpmn-io#2187](bpmn-io#2187))
* `FEAT`: prioritize start of tokens in search results
([bpmn-io#2187](bpmn-io#2187))
* `FIX`: do not commit viewport changes on `ESC`
([bpmn-io#2189](bpmn-io#2189),
[bpmn-io#2187](bpmn-io#2187))
* `DEPS`: update to `diagram-js@14.10.0`

## 17.10.0

* `CHORE`: correct various type hints
([bpmn-io#2228](bpmn-io#2228))
* `FIX`: pasting compensation activity without boundary event
([bpmn-io#2070](bpmn-io#2070))
* `FIX`: lane resize constraints for se and nw direction
([bpmn-io#2209](bpmn-io#2209))
* `FIX`: auto place elements vertically in sub-processes
([bpmn-io#2127](bpmn-io#2127))
* `FIX`: hide lane label during direct editing
* `DEPS`: update to `diagram-js@14.9.0`

## 17.9.2

* `FIX`: keep direction when collapsing pools
([bpmn-io#2208](bpmn-io#2208))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikku nikku nikku approved these changes

@jarekdanielak jarekdanielak Awaiting requested review from jarekdanielak

@barmac barmac Awaiting requested review from barmac

Assignees

@philippfromme philippfromme

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@philippfromme @nikku