Enterprise-ng: Fix `settings` assignment for components

[bthharper]

Describe the bug

Many of the components allow the "whole" settings (options) object to be set via an Input or via a initialize method, however this replaces the settings in the component in its entirety.

The component should merge the options in with the defaults (as provided by the underlying jQuery component).

This is not a problem when setting the options individually, however using the full setter may cause problems.

To Reproduce

This can be seen when setting the options on ContextualAccessPanel, SohoModal and SohoModalDialog.

This is fixed on ContextualActionPanel - but wanted to raise this issue to ensure other components are reviewed.

Here are a couple of examples:

1. SohoModal overwrites the options on creation:

options(options: SohoModalOptions): SohoModalRef<T> {
  this._options = options;
  // @todo update the dialog if required.
  if (this.modal) {
    this.modal.settings = options;
    // @todo - need an api on modal to update settings.
  }
   return this;
}

1. DatePicker overwrites the options via an Input:

@Input() set options(options: SohoDatePickerOptions) {
  this._options = options;
  if (this.datepicker) {
    this.datepicker.options = this._options;
  }
}

Expected behavior

When assigning settings in one call, the settings should be merged into the component (and widget) rather than over writing the settings.

Version

• ids-enterprise-ng: 6.4.x+

Screenshots

N/A

Platform

N/A

Additional context

N/A

[tmcconechy]

A lot of the core components have a updated method that should do the merge and update based on the changes. Some do a tear down and init again but some just update in place as needed. We could probably try and call updated(settings) for a fix for this and then address any issues.

[EdwardCoyle]

A good practice in the NG components, generally, would be to simply do what Tim's saying and never directly change an internal settings object on an EP component. Many of the EP components now have internal processes for normalizing types/values within settings that may conflict with those kinds of direct setters. Using [EP Component].updated(settings); is almost always preferred.

[bthharper]

Agree, most of the "problem" components are old ones before the updated method existed.

The newer components follow this pattern:

• update a local settings object with the new value
• set a member variable indicating an update is required
• call markForCheck on the Angular change detector
• call updated on the wrapped jQuery widget in the ngAfterViewChecked lifecycle event if the member variable is set.

So we follow that.

[tmcconechy]

Closing as too difficult for the present version but in the 5.0 version thats in the works each individual property will update what it needs to. So will be solved on 5.0.0 when done