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

Add a refresh function to the popupMenu object #804

Closed
marcellobarile opened this issue Sep 19, 2023 · 9 comments · Fixed by #860
Closed

Add a refresh function to the popupMenu object #804

marcellobarile opened this issue Sep 19, 2023 · 9 comments · Fixed by #860
Assignees
Labels
enhancement New feature or request

Comments

@marcellobarile
Copy link

marcellobarile commented Sep 19, 2023

Is your feature request related to a problem? Please describe.

With https://github.com/camunda/web-modeler/issues/6066 we are adding to the web-modeler the capability to import element templates from the marketplace;
once this is done via a pop-over dialog, the user should go back to the diagram following a non-interrupting UX; the just imported element templates should be listed in the element templates list while it's opened.

Describe the solution you'd like

We are hacking the popupMenu in the web-modeler by programmatically closing and re-opening it.
This solves the issue because it implicitly forces a new rendering.

This could be avoided if the popupMenu was exposing a refresh method that computes and renders the entities list from scratch.

In the alternative, the list in the UI could be internally refreshed as soon as new templates are set via the elementTemplatesLoader.

Additional context

First, we update the list of templates by calling elementTemplatesLoader.setTemplates as soon as we have imported new element templates.

Secondly, this is the hacky solution implemented in the web-modeler to "propagate" the change also to the UI (without having to go away from the diagram, or unselecting and reselecting the element and other actions that might create friction during the modelling):

try {
    if (popupMenu.isOpen()) {
       const { target, position, options, className: providerId } = popupMenu._current;
          
       popupMenu.close();
       popupMenu.open(target, providerId, position, options);
    }
} catch (err) {
    console.log(err);
}
@marcellobarile marcellobarile added the enhancement New feature or request label Sep 19, 2023
@marcellobarile marcellobarile changed the title Add a refresh function to the popMenu object Add a refresh function to the popupMenu object Sep 19, 2023
@barmac
Copy link
Member

barmac commented Sep 19, 2023

If I understand you correctly, the required feature is to be able to reload the content of the popup menu without reopening.
Given a popup menu is open, when an external system triggers refresh, the popup menu should retrieve the entries again and re-render its contents. Is that correct?

Still, I can imagine that even such expansion of the API may appear to be too limited soon. If a user requests additional entries which are loaded asynchronously, they may expect some kind of loader/spinner for which there is no place in the current, synchronous API. Perhaps we need to have a popup menu which could receive entries asynchronously? Or a way to hook up into what component is displayed?

@marcellobarile
Copy link
Author

@barmac

Is that correct

Correct :)

I can imagine that even such expansion of the API may appear to be too small soon. If a user requests additional entries which are loaded asynchronously, they may expect some kind of loader/spinner for which there is no place in the current, synchronous API

My gut feeling is that this depends on the direction that we want to take in the future; right now the async business logic to select / fetch / import new templates is handled by different components of the web-modeler, meaning that when we set new templates in the popupMenu we already have everything in place.

@nikku
Copy link
Member

nikku commented Sep 19, 2023

To me this sounds like a reasonable request. We already use such functionality internally.

@nikku
Copy link
Member

nikku commented Sep 19, 2023

@marcellobarile Maybe you can take inspiration from the built-in refresh functionality.

That could shorten the hack you mentioned.

@CatalinaMoisuc CatalinaMoisuc added the ready Ready to be worked on label Sep 21, 2023
@CatalinaMoisuc
Copy link
Member

There is no capacity in this iteration, we leave it in ready to be prioritized in the next iteration.

1 similar comment
@barmac
Copy link
Member

barmac commented Oct 31, 2023

There is no capacity in this iteration, we leave it in ready to be prioritized in the next iteration.

@philippfromme
Copy link
Contributor

@barmac You are assigned to this issue. Are you planning to work on it anytime soon?

@philippfromme
Copy link
Contributor

@barmac As discussed, I will take over this issue.

@philippfromme philippfromme added the in progress Currently worked on label Feb 21, 2024 — with bpmn-io-tasks
@philippfromme philippfromme removed the ready Ready to be worked on label Feb 21, 2024
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Feb 21, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Feb 21, 2024
@philippfromme
Copy link
Contributor

Closed by #860.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants