References / Select Editor Redone

[skurfuerst]

References / Select Editor

<- must-have for 1.0.0

the current Pull Request (WIP) can be found at #726

This document is divided into three parts:

• Use Cases, detailing the requirements for the Select/References editor (Problem Space)
• Problems with current implementation, reasoning what we learned from building the current implementation
• Implementation Idea - the new implementation idea

Use Cases

The Select component is used in a variety of ways in the UI:

• as a "simple select" to choose from predefined options in various UI parts

• in the inline link editor, where the user starts typing and gets an async list of link suggestions

• in the reference+link editors, where the user starts typing and gets an async list of reference suggestions

• in the select editor:

  • choose between predefined options
  • choose between predefined options, but with search
  • choose between async loaded options, which are all loaded on startup (dataProvider)
  • [would be nice, but not possible now] choose between async loaded options, which are all loaded when the user types

• in the MultiSelect editor:

  • choose between predefined options
  • choose between predefined options, but with search
  • choose between async loaded options, which are all loaded on startup (dataProvider)
  • [would be nice, but not possible now] choose between async loaded options, which are all loaded when the user types
  • allow to sort multiple selections

In all cases, the select value bound from the outside is not the label shown in the select box; but often some kind of identifier (i.e. node identifier). Sometimes, the select box includes the "empty" value, sometimes not (depending on the configuration).

In all cases, if an already-selected option value is not existing anymore, show [invalid value: XY].

In the async cases, we need to show a loading indicator.

Asynchronous loading

It turns out the synchronous cases are all rather easy; it's the async cases which are difficult. We need to distinguish between the following cases:

• Pre-populate the list options on load; and if something is selected, also fetch the label for the selected ID. Pseudo API: getOptions() returns Promise
• Load the options lazily on typing (autocomplete). Here, the initial case needs to be handled separately, as we need to resolve by ID and not by label: getSelectedValuesInitial(IDlist) returns Promise getOptions(SearchString) returns Promise

Problems with the current implementation

The current version of the Select Box is already the second bigger iteration. In the first iteration, we had just a single stateful component dealing with all aspects of the implementation.

In the second iteration (which is currently in master), we tore apart different components, leading to the structure shown in the linked diagram:

open DIAGRAM - current implementation

The problem is that complexity grows out of hand when using an asynchronous MultiSelect editor, as on the one side, the MultiSelect is responsible for loading options; but on the other side, it embeds the (single) SelectBox, which is also responsible for loading options.

This makes it hard to implement the full behavior in a consistent manner.

Additionally, if people use nodes with lots of Select Editors (data providers or references); currently there is a request triggered per editor for loading the options; and previous results are not cached at all.

---

Implementation idea

Basic idea:

• Make the components completely stateless
• Completely move the asynchronous loading functionality to Redux; not triggered by the component itself, but rather by a saga (which knows specifically when asynchronous data must be loaded; and which can batch same-time loads together into one request). This separates the concern of data loading from the "rendering" concern.
• cache the option-values in the Redux store, using some kind of LRU (Least Recently Used) mechanism.
• Use composition over inheritance.

Component Architecture

The diagram linked below shows the proposed component and state architecture.

open DIAGRAM - proposed component architecture

Base Components

All base components will inherit from PureComponent, and be stateless.

<SelectBox
  value={the currently selected value}
  onValueChange={callback(newlySelectedValue)}
  options={an array of option objects; where each option "value" property is matched against "value"}

  // helper for asynchronous loading; should be set to "true" as long as "options" is not yet populated.
  displayLoadingIndicator={true|false}

  // search box related properties
  displaySearchBox={true|false}
  searchTerm={the current search term}
  onSearchTermChange={callback(newSearchTerm)}

  // override option rendering
  optionRenderer={option => <RendererForItemInOptionsList />}
  />

• the SelectBox does not care whether it is asynchronous or not; this is all done by the higher components when connecting with the state

<MultiSelectBox
  values={the currently selected values (array)}
  onValuesChange={callback(newlySelectedValues)}
  options={an array of option objects; where each option "value" property is matched against "value"}
  allowReordering={true|false}

  // helper for asynchronous loading; should be set to "true" as long as "options" is not yet populated.
  displayLoadingIndicator={true|false}

  // search box related properties
  // displaySearchBox is always "true"
  searchTerm={the current search term}
  onSearchTermChange={callback(newSearchTerm)}
  displaySearchLoadingIndicator={true|false} // there needs to be a separate loading indicator for the search input field

  // override option rendering
  optionRenderer={option => <RendererForItemInOptionsList />}
  />

State Structure / Reducers / Sagas

When thinking about the State structure, you might ask yourself why we should not just use a stateful React component containing the state. I think there are multiple answers to this why that's a bad idea (for me at least):

• In our current (v2) implementation, that's roughly what we do; and it increases complexity tremendously. I personally think it is the main contributor to why the Select compexity grew so much.
• It means we need to start and control AJAX calls directly from React components; including all possible error behaviors.
• It makes it impossible to f.e. re-use already loaded data. If a data provider is used a) in multiple editors for the same node; or b) across multiple nodes (and its return value is not node-dependent), then it would be useful to just load the data once and cache it for some time. I believe this has the potential to make the UI a lot more snappy.

In my opinion, the data flow should be as unidirectional as possible. This means: It should not be a component's responsibility to determine whether some data it needs is missing (e.g. option labels). Instead, to me, that are different concerns; neatly encapsulated in Sagas.

How would the State be structured? I propose the following:

• Top-Level Key: UI.asynchronousValueCache.<CachePartKey>
• The CachePartKey is an identifier, containing:
  • Inline-Link for inline links
  • DataProvider-<DataProviderName>-<NodeIdentifier>
  • in the longer run, it would be useful to be able to mark a data provider as "not being dependent on the current node", then we could skip the Node Identifier from the Cache Key; increasing the hit ratio.
  • NodeReference-<NodeTypeListAllowedAsReferenceTarget>
  • Asset for Asset editor
• Each "cache part" looks as follows:
  • valuesByIdentifier: an Identity Map, where each key is the "identifier" (f.e. the Node Identifier) for the object; and the value is an JSON structure describing the object - like {label: "foo", previewImage: "...."}. This is dependent on the cache type.
  • searchStrings: A map which maps each "search string" the user has entered to a list of identifiers returned by the search.

A full example looks as follows:

UI.asynchronousValueCache.NodeReference-Neos.Neos:Document: {
  "valuesByIdentifier": {
    "dd118fff-5d1c-4fec-82a9-aef33df21447": { "label": "Homepage" },
    "82dc8a1d-6097-40d2-bf79-1fedbd6d9aed": { "label": "Homer Simpson" },
    "9f52e19c-677e-4a70-be73-daa7d99c8c56": { "label": "Neos CMS" },
    "ed7c2575-4a7a-4dfb-bec6-2f19abb53997": "LOADING" // "LOADING" is a special key meaning we are currently loading the values.
  },
  "searchStrings": {
    "": ["dd118fff-5d1c-4fec-82a9-aef33df21447", "82dc8a1d-6097-40d2-bf79-1fedbd6d9aed", "9f52e19c-677e-4a70-be73-daa7d99c8c56"],
    "H": ["82dc8a1d-6097-40d2-bf79-1fedbd6d9aed", "dd118fff-5d1c-4fec-82a9-aef33df21447"],
    "Home": ["82dc8a1d-6097-40d2-bf79-1fedbd6d9aed", "dd118fff-5d1c-4fec-82a9-aef33df21447"],
    "Homep": ["dd118fff-5d1c-4fec-82a9-aef33df21447"],
    "Ne": ["9f52e19c-677e-4a70-be73-daa7d99c8c56"],
    "Neo": "LOADING" // "LOADING" is a special key meaning we are currently loading the values.
  }
}

The LRU mechanism

Actually, the example above is not fully correct; as cache size might grow quite quickly; and never shrink.

Thus, we need a mechanism to evict caches; and we suggest to use a variant of LRU (Least Recently Used) here. If the cache is not used 10 times in a row, we drop it. In order to implement this, we need a notion of how to count cache usage; which is application-specific.

As an idea, we always increase the "cache usage counter" when the currently active node is changed; or when a new node is inserted (because the creation wizard might also contain select boxes and async loading).

I'd propose to do the LRU mechanism on the level of Cache Parts, and NOT on the level of individual cache entries; as otherwise the bookkeeping effort (and possible time to update the LRU counters) will be quite huge.

As a single Cache Part could then grow unconstrained, the idea would be: If it reaches a certain size, we drop all entries except the ones we need for the current point in time.

Loading options through Sagas

The actual option-loading (including the LRU mechanism) should be controlled and triggered by Sagas.

Example / Pseudo Code:

Saga is triggered:
  - ON Node Change
  - ON Node Creation Dialog Step 2 Shown
  - ON SelectBox Editor SearchValue Changed
  - ON Inline LinkEditor visibility changed

PSEUCODODE:
- determine which options should be loaded:
  - start with the currently selected Node Type
  - which properties are of type reference(s) PLUS
  - which properties are of type Select with DataProvider PLUS
  - which properties are of type assets PLUS
  - which properties are of type link
- for each to-be-loaded-option, do the following:
  - figure out which CachePartKey should be used
  - figure out whether valuesByIdentifier exist for the currently-selected value. If not, start loading them.
  - figure out whether searchStrings exist for the currently-selected value. If not, start doing the AJAX query.
  - After the load completed, set LRU counters for the used CachePartKey to 0.
  - Increase all other CachePartKey LRU counters. Remove CacheParts which have a too-high LRU counter.

Probably we won't create a single Saga, but multiple of them; and move out common utility functions for code reuse.

Reference/References Editor

The reference editor basically needs to connect to the Redux Store; and calculate the CachePartKey.

Select/MultiSelect Editor

we probably have one editor, which, depending on defined options, will delegate to nested editors which then will be connected to the Redux Store.

Link Editor (Inspector)

Inline Link Editor

Estimation

(how much work is it to implement this?)

I'd guess 1-2 Weeks in total.

Further Work

• Apply the same principle on the Image Editor

[dimaip]

Niiice! Thanks a lot for thinking it through in such detail! Totally agree to making simple stateless components, without inheritance and confusing mixing of presentation and logic that we have now.

I'm not so sure of the benefits of putting the components state into Redux/Sagas. A higher level component that would encapsulate the state could work too. How would the shape of the Redux state look and where would it be put for all of the usecases that we have? These editors have to be rather self-contained, e.g. so we could put then inside inspector, inside node creation wizard, as structured inline editors and so on.

[skurfuerst]

@dimaip I added some sections about the state structure; detailing it a lot and hopefully answering your questions :)

Now this concept is not WIP anymore, but ready for full-blown discussion :)

[gerhard-boden]

@skurfuerst Thanks a lot for creating this Issue with all the in-depth descriptions and explanations! Can't believe your created all that in an hour! This is the ideal way how to discuss topics like this! Everyone who is interested can quickly get some insight knowledge how things currently are and how you propose them to be in the new UI.

I really like the concept, here is some feedback / questions:

• "Load the options lazily on typing (autocomplete). Here, the initial case needs to be handled separately, as we need to resolve by ID and not by label:"

  I though we always solve by ID (=== option value?), because that would be the value the node property holds, or am I mistaking?

• "useful to just load the data once and cache it for some time."

  I really like the idea and the advandage of having cached data shared across multiple select fields. I'm trying to think of scenarios where this might be dangerous, but I think we'll be okay :) Since there is one (shared) data source this could also open the possibilty for options in multipe (indepentet) select fields, where an option may only be applied once (and in no other field at the same time). This of course can be implemented sometime in the future when it's even useful to someone, because basically we can already do this with a single MultiSelect.

• Make the components completely stateless

  Hooray for small stateless components :) Can it also be a functional stateless component, since we won't need state or lifecycle methods there? Could also become a great starting point for React newcomers who want to see how lightweight a custom component in the React UI can be.

• The LRU mechanism

  This part I do not understand "If the cache is not used 10 times in a row, we drop it." What counts as usage? If every single letter creates a separate cache entry, the cached entry I could have needed might already be dropped until I've reached my full search term?

[skurfuerst]

Hey @gerhard-boden,

actually it took more like 3 hours to create all that; but I guess it's worth it ;)

Question 1: Well, when loading a dropdown initially we select by ID (f.e. Node Identifier). However, when you start typing something into the field, we actually do a search "by value" (f.e. Node Label).

Question 3: We'll see :-) That's something I'd decide when implementing the components.

Question 4 (LRU): A "Usage" is the following:

As an idea, we always increase the "cache usage counter" when the currently active node is changed

So basically, it means we cache everything for the last 10 visited nodes.

All the best, Sebastian

[aertmann]

Hey a little late to the party. Your proposal looks pretty solid @skurfuerst, well done! And awesome to see some of these issues we've been wanting to solve in the old UI being solved now. Goes very much in line with some of the ideas for improvement I personally had while working on the old stuff.

Definitely see a great benefit from caching the fetched data for re-use across similar editors. Now whether that's better done using redux or a re-useable stateful component I'm not sure. I think there's a fine line depending on which editor and how the options are calculated. E.g. if they are calculated server-side, it's expensive, however if it's filtering an array of already available data, it's less expensive. E.g. node type editor uses data already available, same goes for a data source that loads all options (here it makes sense to store those for re-use). However a data source that needs to be calculated server side during search is expensive, but also difficult to cache (unlikely much useful re-use). If everything is treated the same way using LRU it might result in a not so optimal solution overall, thus maybe differentiating between cheap and expensive and applying the LRU cache on the expensive ones with high chance of re-use make sense.

Also regarding the dataProvider, in the old UI it's even possible to add additional data to the provider in addition to the node, which would potentially complicate that cache identifier further.

You don't mention the plugin editors, but wanted to mention that in the old UI they're also based on the select editor (MasterPlugin, PluginView, PluginViews).

Just for the completeness of this issue, wanted to mention some of the features/options available in the old UI to keep track of it in one place.

• Grouping of options
• Sorting of options
• Icon for options
• Disabled of options
• Search placeholder & search threshold

Lastly you mention the asset editor in the state/structure section, but don't include it in the listing of editors in the end. Do you intend to base the asset editor on these components as well or is that a separate topic? In the old UI it could be build using it, but totally rolls it's own implementation. If I were to implement it again I would probably go a different route all together and create a asset list editor with support for file selection, file upload and image cropping instead of reimplementing the asset list, knowing that's totally out of scope here.

[skurfuerst]

Thanks @aertmann for your detailed feedback and comments -- now as I started implementing this I am also thinking more detailedly about the different loading-use-cases (which you have explained very well)!

The point I am currently unsure about is:

Should the Saga figure out by itself if data needs to be loaded (uni-directional data flow); or should the SelectBox say "you know, I am currently being rendered and I need a label for my value "XYZ"

Conceptually I prefer the first solution somehow, but while implementing it it seems it might actually be more difficult than the second one; and both get the job done. I'll think some more about their consequences :)

I'll try to update the Pull Request today :)

All the best, Sebastian

[skurfuerst]

FYI: The second solution works nicely!

[skurfuerst]

• How to invalidate the caches if a new page is inserted?

[skurfuerst]

implemented by now :)