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

Integrate new replace menu #3332

Closed
2 tasks done
Tracked by #1627
nikku opened this issue Dec 2, 2022 · 7 comments · Fixed by #3364
Closed
2 tasks done
Tracked by #1627

Integrate new replace menu #3332

nikku opened this issue Dec 2, 2022 · 7 comments · Fixed by #3364
Assignees
Milestone

Comments

@nikku
Copy link
Member

nikku commented Dec 2, 2022

What should we do?

  • add new replace menu
  • add keyboard shortcut (R) to replace currently selected element

Why should we do it?

Support bpmn-io/bpmn-js#1627.

@nikku nikku added the ready Ready to be worked on label Dec 2, 2022
@nikku
Copy link
Member Author

nikku commented Dec 2, 2022

@smbea To consider if we want to do this (+ keyboard shortcut).

@smbea
Copy link
Contributor

smbea commented Dec 2, 2022

Basically this means bumping versions and adding the keyboard shortcut, right?

@nikku
Copy link
Member Author

nikku commented Dec 2, 2022

Yes. The keyboard shortcut may be missing with stock bpmn-js (yet). Did we add it?

On the electron side of things we need to add it as a menu item, in addition.

@smbea
Copy link
Contributor

smbea commented Dec 2, 2022

We haven't added it yet in bpmn-js, no. It is at the end of the checklist here bpmn-io/bpmn-js#1627 but since we decided to ship just replace, it should've been split up too oops

@nikku
Copy link
Member Author

nikku commented Dec 2, 2022

Could you split up that monstrous issue up in two parts (ship replace + rest) @smbea?

Makes it easier to follow along and not miss out on such things.

@smbea
Copy link
Contributor

smbea commented Dec 4, 2022

Question: The 'R' shortcut overlaps with the token simulation reset shortcut. Isn't this something we should avoid?

@nikku
Copy link
Member Author

nikku commented Dec 5, 2022

Let's not care about the token simulation R shortcut for the moment, but rather move forward at the core. Token simulation (similar to all other extensions) have to adopt. I.e. could decide to ditch the shortcut or use a different one to ensure 100% interoperability.

A simple way to solve this is by making R contextual, which it already is, I think:

  • R shall be reset when token simulation is active (only)
  • R shall be replace when it is not active

@nikku nikku added this to the M60 milestone Dec 9, 2022
smbea added a commit that referenced this issue Dec 15, 2022
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed ready Ready to be worked on labels Dec 15, 2022
@smbea smbea linked a pull request Dec 15, 2022 that will close this issue
smbea added a commit that referenced this issue Jan 2, 2023
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jan 2, 2023
smbea added a commit that referenced this issue Jan 2, 2023
smbea added a commit that referenced this issue Jan 2, 2023
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Jan 2, 2023
@nikku nikku modified the milestone: M60 Jan 5, 2023
lzgabel pushed a commit to lzgabel/camunda-modeler that referenced this issue Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants