improve toolbar more configurable

Description

We discussed that on the iDempiere workshop 2019.

showed us his toolbar. It looked quite good. He has both icon buttons and text buttons. You have only some buttons shown at the left and a "more" drop down menu at the very right. And he also has a combobox for searches.

The audience agreed that Norberts toolbar is nice and we should open a ticket to add his changes.

Environment

None

Activity

Show:
Diego Ruiz
February 17, 2021, 4:53 PM

Hi guys,

 

I’ve created a new pull request.

 

What I’ve noticed is that the updateToolbarAccess runs only once because of the ToolBarMenuRestictionLoaded variable (even if called multiple times), which means the window access logic could be easily moved to the init method and only append buttons that will be used at some moment in the window, instead of appending and then removing. This is exactly what the pull request does.

It also has the benefit that it is independent of if it is mobile, desktop, Combobox, button, it should always work independently from the UI structure, which is a big plus IMHO.

 

Two things to note here:

  1. The parameter (xAD_Window_ID) in the updateToolbarAccess method is never used, I believe it was used at some point in time, then the method logic was changed but the parameter was never removed. Didn’t trace when this changed happened but it was a long time ago. I want to propose to remove that parameter from the method, it is used only at one point in Core, and I do not know how probable is that someone is calling that method from a plug-in because of the high dependency on ZK and the UI structure, but I didn’t want to remove it without consulting it first with everyone.

  2. This solves the issue that addresses above. I believe the same issue might be present in dynamicDisplay when navigating to a detail tab (if it is excluded in the detail tab but not in the whole window the dynamic display is not working for showmore buttons) but I haven’t checked how to address that, when I do, I will create a new pull request so it is simpler to review and test

 

Regard,

Diego Ruiz

Diego Ruiz
February 16, 2021, 12:34 PM

Hi ,

 

The commit doesn’t work on mobile. You can reproduce the same steps that you described in your comment on mobile and you can still see the two buttons (export and Import File Loader)

 

Additionally, I think that was the original goal of this if statement

 

 

This commit changed the approach from Menupopup to Popup but didn’t update the validations, so I think instead of adding a new else if, it should replace this one, that code segment is never reached so it can be removed. Probably need to replace it with a similar if statement to the one you added to every Menupopup validation (updateToolbarAccess and DynamicValidation).

 

Best Regards,

Diego Ruiz

Carlos Ruiz
February 16, 2021, 10:59 AM

Found that the advanced buttons that are in the Show More area are shown also for non-advanced roles.

Test case:

  • Role GardenWorld Admin Not Advanced

  • Open Freight Category window

  • Check the Show More area of the toolbar

  • ERROR: The buttons Export and Import File Loader are displayed, those buttons are advanced and the role is not advanced

This is a security issue.

Carlos Ruiz
February 20, 2020, 3:14 PM

Committed

Diego Ruiz
February 20, 2020, 2:39 PM

Thank you ,

I misinterpreted your first message and thought the problem was on mobile. Please see the attached patch that solves the issue you describe.

Fixed

Assignee

Diego Ruiz

Reporter

Thomas Bayen

Labels

Tested By

None

Components

Fix versions

Priority

Minor