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

Prevent duplicated help menu entries #3044

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

pinussilvestrus
Copy link
Contributor

@pinussilvestrus pinussilvestrus commented Jul 21, 2022

Originated from #3035.

This prevents duplicated help menu entries in the backend.

Closes #3032

Copy link
Contributor

@barmac barmac left a comment

Choose a reason for hiding this comment

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

If we merge this PR, no link will be displayed when --disable-platform flag is set. We already don't display tutorial links for BPMN in such a situation, which I think could be considered as a bug as well.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Jul 21, 2022
@barmac
Copy link
Contributor

barmac commented Jul 21, 2022

So perhaps we should display/hide links based on the --disable-<file-type> flags, not based on provider appearance.

@pinussilvestrus
Copy link
Contributor Author

Very good catch 👍 I will propose something.

@pinussilvestrus
Copy link
Contributor Author

What about ensuring the help menus only got added once in the TabsProvider:

const dmnProvider = (this.providersByFileType['dmn'] || [])[0];
if (dmnProvider) {
  dmnProvider.getHelpMenu = () => DMN_HELP_MENU;
}

const bpmnProvider = (this.providersByFileType['bpmn'] || [])[0];
if (bpmnProvider) {
  bpmnProvider.getHelpMenu = () => BPMN_HELP_Menu;
}

This way, we don't depend on --disable-platform | --disable-zeebe flags. Looks a bit like monkey-patching, but we are anyway manipulating the providers via flags this way.

@barmac
Copy link
Contributor

barmac commented Jul 21, 2022

This should solve the issue. We could also implement it on the backend: https://github.com/camunda/camunda-modeler/blob/develop/app/lib/menu/menu-builder.js#L549

@pinussilvestrus
Copy link
Contributor Author

As the backend does not know about BPMN, or DMN, we could definitely filter for duplicates, e.g. via label, or label + action.

Niklas Kiefer and others added 2 commits July 25, 2022 10:12
Co-authored-by: Pawel Kurek <pawel.kurek975@gmail.com>
@pinussilvestrus pinussilvestrus force-pushed the 3032-pawel975-duplicated-dmn-tutorial branch from 683dcea to 3cebd60 Compare July 25, 2022 08:14
@pinussilvestrus pinussilvestrus changed the title fix(client): remove duplicated DMN tutorial Prevent duplicated help menu entries Jul 25, 2022
@pinussilvestrus
Copy link
Contributor Author

I updated the PR to prevent duplicates in the backend. Added @smbea as any other is on vacation 🙂

Copy link
Contributor

@smbea smbea left a comment

Choose a reason for hiding this comment

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

I think this looks good 🚀

@pinussilvestrus pinussilvestrus merged commit 3100fee into master Jul 26, 2022
@pinussilvestrus pinussilvestrus deleted the 3032-pawel975-duplicated-dmn-tutorial branch July 26, 2022 07:41
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Jul 26, 2022
pinussilvestrus pushed a commit that referenced this pull request Aug 5, 2022
philippfromme pushed a commit that referenced this pull request Aug 5, 2022
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 this pull request may close these issues.

None yet

3 participants