abp icon indicating copy to clipboard operation
abp copied to clipboard

Assumption: settings tab is loaded on initialization

Open ayyash opened this issue 1 year ago • 5 comments

In settings manager package: SettingManagementModule package I chose to load it in an already lazy loaded module to relief the main bundle and remove unnecessary components, so all my settings tabs are added lazily in a new module.

The following line however, assumes that the settings tabs is prefilled on load of application. It displays the navigational element when at least one permission of the settings tabs is positive. That is too optimistic.

https://github.com/abpframework/abp/blob/e6e91bf11cd219ccab72633b13994dc5be2d2932/npm/ng-packs/packages/setting-management/config/src/lib/providers/visible.provider.ts#L25

export function setSettingManagementVisibility(injector: Injector) {
  return () => {
    const settingManagementHasSetting$ = injector.get(SETTING_MANAGEMENT_HAS_SETTING);
    const isSettingManagementFeatureEnable$ = injector.get(SETTING_MANAGEMENT_ROUTE_VISIBILITY);
    const routes = injector.get(RoutesService);
    combineLatest([settingManagementHasSetting$, isSettingManagementFeatureEnable$]).subscribe(
      ([settingManagementHasSetting, isSettingManagementFeatureEnable]) => {
        routes.patch(eSettingManagementRouteNames.Settings, {
          // settingManagementHasSetting is not necessarily true at the time this function runs
          invisible: !(settingManagementHasSetting && isSettingManagementFeatureEnable),
        });
      },
    );
  };

The visibility check cannot be dependent on a list of components that have not yet been loaded. The work around was to initialize the settings tabs with null components, then patch those tabs when my module is loaded. That shouldn't be this hard. May be there should be a "visibility predicate" on root instead? Let user decide the condition to show the navigation element.

ayyash avatar Feb 04 '24 12:02 ayyash

hi @ayyash, i didn't understand your request exactly. Setting management package is already loading lazily. They are not added to main bundle(initially).

and if we dont give this provider to root injector, i mean if we dont run it in inital time. we cannot set the setting tab visibility.

image

Sinan997 avatar Feb 08 '24 11:02 Sinan997

Hi @ayyash visible.provider stored in config module which is secondary-endpoint it must load inital. The main module is SettingManagementModule itself. I didn't get it which component(s) we depended in the provider please make it simple for us 🙂

masum-ulu avatar Feb 08 '24 11:02 masum-ulu

I understand it must be known initially, but in order to check the visibility flag, it should be something like this (for example)

if (features && permissions) setVisibile(true)

Right now its:

if (features && settingstabs.length) setVisible(true)

The code assumes the settings tabs are full, they arn't, because they are components that will be needed only after clicking on the settings tabs. I found myself having to do in application root:

 settignsTabs.add([
    {
      name: '::Something',
      order: 1,
      requiredPolicy: 'Something.Create',
      component: null, // notice this
    },

And later in the actual settings module:

export class AdminSettingsModule {
    constructor(private tabs: SettingTabsService) {
      
      this.tabs.patch(
        '::Something', {
        component: SomethingSettingsComponent, // now add it
      });

It is an odd way to do it.

ayyash avatar Feb 08 '24 15:02 ayyash

Thanks for your effort. I think you confused about SettingsTab and RouteMenuItem providers. In both case we need to load eagrly.

SettingTab case

In this case when you go to setting-management page, tabs will render by condition and all tab needs to know what is it's component

image

image

RouteMenuItem case

We are not import any component (chunk) here

image

image

masum-ulu avatar Feb 08 '24 18:02 masum-ulu

You're right I was talking about the RouterMenuItem I thought I mentioned that

Because of this though, the EmailSettingGroupComponent has to be declared on the root AppModule. All components inside this menu item must be declared in root, correct? Is there a way to do this without including the components in the root? (besides my work around?)

SettingManagementConfigModule has this component, which must be imported in the AppModule. Unless I'm missing something.

ayyash avatar Feb 09 '24 10:02 ayyash