Problem/Motivation

There have been discussion about advanced settings for this module. Changing things like title, arguments, number of items, pager came up. As we can't keep adding columns forever, we should discuss how to deal with this situation. Some questions:
- What are the settings this module want to support?
- How can we store this data in a generic way?
- How can we make sure modules can easily extend this?

Proposed resolution

I think a custom plugin type could be the answer.

  • Turning plugins on/off could be done in the field settings.
  • Each plugin could define what setting to add to the form and the type of setting (textfield, number, checkboxes whatever) by letting the existing widget call enabled plugins. Maybe the plugin could provide a subform?
  • The plugin could then handle validation for the data in the serialized array we store.
  • The plugin could also handle the changes to the view by letting the existing formatter call enabled plugins and pass the view.

The main module could ship with basic plugins (title and arguments for example to have parity with the existing options). Optionally include submodules with more advanced settings. Other modules could add their own plugins for exotic usecases.

Having a generic 'data' column to store all data on a entity level would make sure this could work and can easily be extended.

Remaining tasks

- Discuss how to move forward
- Create issues to do the actual development

User interface changes

API changes

Data model changes

Comments

seanB created an issue. See original summary.

NewZeal’s picture

You need to define what you mean by plugin. A field formatter is a plugin. In my view the submodule would include a field formatter and a field widget and all the settings under the sun can be added to it.

If we add the data field to the mother ship (viewsreference module) then there is no need to add new fields. The data can be stored in a serialized array. Adding new fields involves adding a new FieldType and that would require users to recreate fields in order to use the new settings.

seanB’s picture

Issue summary: View changes

Sorry about that, I meant a new custom plugin type. The field formatters are set on the entity type and we probably want to control the settings per entity. This also allows other modules to easily extend the options.

Updated the IS.

seanB’s picture

Issue summary: View changes
NewZeal’s picture

Still not sure what you mean by plugin. Do you mean a views display plugin?

The field formatter is set on the field, not the entity type. This allows for settings per entity. We can currently alter settings for title, views plugin and arguments per content entity. Nothing you are suggesting appears different to what has been implemented.

How will other modules "extend the options" in your suggestion of how this might work?

seanB’s picture

I meant building a custom plugin manager, discovering a custom annotation based plugin type:
https://www.drupal.org/docs/8/api/plugin-api/creating-your-own-plugin-ma...
https://www.drupal.org/docs/8/api/plugin-api/annotations-based-plugins

The field formatter is set for a field in a display mode on entity type level. This means that the formatter has the same settings for node A and node B of the same node type within the same display mode. I think we want to have separate settings for node A and node B, even if they are the same type. For example you have a page content type with a viewsreference field. Sometimes you want to show 10 items, sometimes 20. I don't think a formatter can do that.

If we want to allow different settings for each entity that contains a field, there are 3 important things:
- Storage (database table/field/columns)
- Data entry (field form widget)
- Data display (field formatter)

Storage
Current situation:
separate fields/columns in the database table for the field are hard to extend. Each new settings requires a new field/column in the database. This makes it hard to add settings.

Proposal:
This was already mentioned in other issues. Having a data field/column in the database table that contains a serialized array makes it easy to add/remove data for each separate entity. Imho existing columns containing settings should be removed and the data should be moved to this data field/column.

Data entry / Data display
Current situation:
The ViewsReferenceTrait hardcodes all fields for settings. Other modules can alter the form to add settings to the widget. The data display is done through the ViewsReferenceFieldFormatter. The only way to change this is adding a other formatter (or maybe implement a hook to alter the view). If several modules want to add and change settings this could be a problem.

Proposal:
It's would be nice to have a way to place all code needed to add a settings in one place. This means defining storage, data entry and data display. To do this, a custom plugin manager could look for implementation of an annotation based plugin type. The widget and formatter could use the plugin manager to add changes to the widget form for data entry and to change the view for data display . Each plugin could for example implement the following methods:

  • buildForm()
  • validateForm()
  • alterView()

The viewsreference module could provide some basic plugins for supported settings. Other modules could add a setting( add fields to the form widget and change the view in the standard formatter) by just writing 1 annotation based plugin class. Doesn't get easier then that.

One extra thing
It would be nice to configure which settings to show for a field instance. For example you don't want the field for the title added to the widget. This could be done by changing the field type ViewsReferenceItem. The field type could use the new custom plugin manager to fetch all plugins. We could allow sitebuilders to enable/disable each plugin for the field instance. When a plugin is disabled, you don't call it in the widget or the formatter.

seanB’s picture

I could provide a patch for these changes if you would consider adding that? I don't think it is a lot of work.

NewZeal’s picture

The field formatter is set for a field in a display mode on entity type level. This means that the formatter has the same settings for node A and node B of the same node type within the same display mode. I think we want to have separate settings for node A and node B, even if they are the same type. For example you have a page content type with a viewsreference field. Sometimes you want to show 10 items, sometimes 20. I don't think a formatter can do that.

Sean, as explained in #5 the formatter in its current state allows you to configure view name, display id, title and arguments per entity. That means that node A and B can have different settings. The data field I have just added will allow many other settings per entity. The only settings done at the entity type level are the controls over which display types are available to choose from and, for the select field, which views to preselect.

In response to the suggestion on how to handle advanced settings, I'm not sure that creating a new injected plugin for each setting in the backend of viewsreference is very good use of code. That kind of plugin is usually reserved for something more consequential. Invoking a hook would be a more appropriate way to handle it, such as using form_alter to supply further settings into the form.

For the viewsreference module adding the data field is all that is required to make the advanced settings module possible. Devs can now create advanced formatters by extending it or simply use a hook to add more settings to the field.

seanB’s picture

The widget allows you to set the title, argument etc. on entity level: ViewsReferenceSelectWidget through ViewsReferenceTrait

I don't see any code in the formatter ViewsReferenceFieldFormatter for this? Maybe we're confusing widgets and formatters?

I think for any new setting you need to change the widget and the formatters. That means at least 2 hooks to get it done. I think these situations is exactly what plugins are for. But if you disagree there is nothing I can do =)

joekers’s picture

Issue summary: View changes

Could we not keep the advanced settings as they are but change the storage so that they are stored in the new serialised data field. I think we want to keep settings on a per entity basis but this is what we already have.

I think it would be useful to control which settings are used in the widget on a per field basis, so you could have a field on a node that allows for more advanced settings, whereas a different field on a paragraph bundle you might want to strip it back and not show any advanced settings.

Also there's only going to be a definite amount of useful advanced settings you'd want to use - building a plugin manager for this might be overkill?

seanB’s picture

I think we all agree on changing the storage to a data column to allow more flexibility. So that's great!

There are different ways to solve the data entry / data display in the widget and formatters. You can basically go for hooks, event and plugins (probably other things when you get creative :).

The thing I like most about plugins is the fact that all code is in 1 file. This is easy to document. Plugins can use DI and are easy to test. You can also force developers that change the widget should also implement a method to change the formatter. Not sure what you mean by overkill? I don't think there is a big difference between hooks, events or plugins? Plugins are used for blocks as well so the pattern shouldn't be hard for D8 developers. I would argue that needing multiple hooks to add a setting is actually more complex and easier to do wrong.

The amount of "useful" settings is limited, but I don't think that's a restriction for using plugins.

NewZeal’s picture

Ok, so we are agreed that the current formatter allows us to change the title, display id and arguments on a per entity basis and there is no need to revise any of that?

As for the new data field, I have included that simply so that we can cater for more settings moving forward. I don't think we should move the title, display id and arguments to the new data field, because that will require further updates to people's code with further risk of breaking the internet. Also, those settings are sufficient for most peoples' usage of this module. Any further settings would belong to another module.

As previously stated, the reason for moving the data field into the viewsreference now is so that for the advanced module, the same FieldType can be used, and people can upgrade the (API) fields that they are currently using rather than create new (API) fields. There is no need to add new settings to the viewsreference module; that is for the submodule. Those settings will become available via a new widget and new formatter.

Sean, the Widget provides fields (columns) on the form field where users can apply settings per entity. The Formatter does the html display after processing those settings.

seanB’s picture

I got confused since you do the actual configuration through the widget, and the formatter uses this. But that's ok, I think we understand each other now. To add settings you need to change both the widget and the formatter, so I guess the extra module should provide at least both.

If I understand you correctly, the main reason to not remove existing columns in favor of the data column is the possiblity that the upgrade path causes trouble. I understand your concern, data loss does "break the internet". I think the upgrade path shouldn't be that hard, but fair enough.

That leaves the part of how to provide extra settings (in a submodule or other modules). If a submodule provides a new widget and formatter in the submodule, would other modules be able to alter the widget and formatters? And if so, do you agree that having all code for each provided setting in a plugin would be the best solution?

NewZeal’s picture

Now that the data field is in the viewsreference module, anyone can write a widget plugin and a formatter plugin to add whatever settings they like, so essentially, what you suggest is already available.

I've tested the update that adds the new field on all the sites that I use viewsreference for and have encountered no problems so will be releasing beta soon.

seanB’s picture

Actually I see a lot of examples: Views uses plugin for a lot of things (arguments, filters etc). Image effects for image styles are plugins. In contrib the metatag module uses plugins to define each type of metatag. Webform uses plugins to provide different elements for forms. The main definition is that it is a 'pluggable component'. I think this fits the description.

But I respect your point of view, thank you for taking the time to discuss this.

NewZeal’s picture

Yes, but are the settings for image styles, arguments and filters done using plugins? Find me somewhere that settings are pluggable, like adding a new setting to a settings panel that is already in place. If you want a new image style plugin setting you create a new image style plugin. In the case of the viewsformatter, I believe the formatter is the lowest pluggable denominator, in that there is no reasonable purpose to going into lower pluggable components.

seanB’s picture

From the top of my head, the conditions for blocks are plugins. CKEditor is a editor plugin that uses other plugins to define buttons etc. In 8.4.x the media module uses plugins to define a media source (local files/youtube video or whatever) for media types, which is also used by the formatters to define how a media item should be displayed.

A lot of settings are pluggable. Image effects are settings of a image style. Arguments and filters are settings of a view. Plugins are a way of providing an interface for a pluggable component, and I think we already established that the settings for a viewsreference field could be pluggable. Providing a hook to change things is also a way of making it pluggable. If we allow other modules to provide settings used by the formatter, this already means the formatter is not the lowest pluggable denominator.

NewZeal’s picture

I've created a submodule called viewsreference_options and pushed to dev. I haven't made any additions and if you want to enable the module you have to uncomment entries in .info file.

If someone wants to create a service and serve settings to this module, then do please provide the code. Meanwhile it should be enough just to add the settings to the relevant plugins to get the more complex views reference module that people are requesting. Based on feature requests, though, I figure that 95% of punters are happy with the basic viewsreference field and do not need any of the options being proposed.

joekers’s picture

Nice work on the submodule, hopefully I'll be able to pull it down and give it a test sometime soon. I guess we'll remove the commented .info once it's ready to be used.

I agree - the advanced settings won't be used by most of our users but I also think that these are one of the things that set this apart from the other version of this module (Viewsfield).

seanB’s picture

I'll try to submit a patch for the submodule to use plugins for advanced settings if you would consider adding it?

NewZeal’s picture

I haven't tested the module code, yet, simply created the minimum files needed to make it work. Obviously this module will be a work in progress for some time while the viewsreference module is moving towards stable release, which is why I disable it in the repo.

Sure, Sean, if you submit your code early, then the advanced module can be based on plugins.

NewZeal’s picture

Status: Active » Fixed

We decided to implement advanced options as a separate module. This is available in the repo in the branch 'options'.

NewZeal’s picture

Version: 8.x-1.x-dev » 8.x-2.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

johnfallen’s picture

Is there any example code for creating a new paragraph with a viewsrefrence field and setting the viewsrefrence->views "type" and display options?

Example:

$paragraph = Paragraph::create([
  'type' => 'my_paragraph_machine_name,
  'field_p_title' => "Show all the things",
 ]);

// Get the view object.
$theView = Views::getView('my_view_machine_name');

if (is_object($theView)) {
   $theView->setDisplay('teaser');
   $theView->save();
   $input = [$theView];

   $paragraph->get('my_viewreference_field')->appendItem($input);

   $paragraph->save();
   $nodeParagraphContainer->appendItem($paragraph);     
   $node->save();
}

That should work, but the value for setDisplay isn't being set or saved. Sorry if I'm missing something glaringly obvious from this thread.

Thanks.