Currently the usage of each plugin's getSettings() method is implemented in a slightly different manner. Compare the more complex FacetapiWidgetLinks::getSettings() method to the simpler CurrentSearchItemText::settingsForm() method. The former you have access to the entire form so you have to know where the widget settings exist in the form structure, whereas the later you just return top level form elements and it places them in the correct location. The later is what we should standardize on.

Regarding the $this->settings variable, again compare the more complex FacetapiWidgetLinks::getSettings() method to the simpler CurrentSearchItemText::settingsForm() method. The former has access to the entire object, where the latter only give access to the appropriate settings. The latter is simpler and improves the DX for plugins.

Comments

cpliakas’s picture

cpliakas commented

This is probably a 2.x task unless we can find a way to make this change backwards compatible.

cpliakas’s picture

cpliakas commented
Status: Active » Postponed

Marking as postponed for the reason mentioned in #2.

cpliakas’s picture

cpliakas commented
Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Postponed » Active

Moving to the 2.x branch.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Issue tags: +API clean-up

Fixing bogus tag.

pwolanin’s picture

pwolanin commented

The methods have different names, so it seems like you could add the simpler method at least to 1.x?

cpliakas’s picture

cpliakas commented

There is a typo in the original post. FacetapiWidgetLinks::getSettings() should be FacetapiWidgetLinks::settingForm(), so the method names are consistent, how they are used is not. I will fix the type in the OP.