Page tree
Skip to end of metadata
Go to start of metadata

What's wrong 

The issues are briefly summarised in  MGNLUI-3490 - Getting issue details... STATUS  and aggregated under MGNLUI-3491 - Getting issue details... STATUS  epic.  This page aims to provide a bit more details and possible ways to improve the situation.

Localisation support

  • Mess with the interfaces, high level of indirection.
    • Transformer#hasI18NSupportTransformedProperty#hasI18NSupport which delegates to the first one.
    • Three objects' methods have to be called so that the new language is applied to a property (TransformerTransformedProperty and DefaultI18NAuthoringSupport):
      • DefaultI18NAuthoringSupport#constructI18NPropertyName;
      • Transformer#setI18NPropertyName;
      • Transformer#setLocale which simply stores new locale in transformer object;
      • TransformedProperty#fireI18NValueChange which in reality simply re-reads the value from the transformer.
  • I18NAuthoringSupport digs into UI structures code searching for the components with localize-able data-sources.
    • Logic separation levels breached - ideally Vaadin-agnostic component crawls the UI hierarchy searching for the property data-sources that can be localised;
    • Looks cryptic and patchy - one has to know how the forms are composed and how the i18n mechanism is implemented in Transformers and TransformedProperty;
    • Rigidness and hacks - instanceof's checking the abstract objects to belong to concrete types like BasicTransformerTransformedProperty etc.
  • Poor support of multi-field localisation 
    • The old property transformers do not support i18n and that probably cannot be fixed due to their implementation;
    • The new "delegating" transformers do support i18n but internally management of locale-specific data is complicated.
  • MGNLUI-3491 - Getting issue details... STATUS  - switching language negatively affects validation.

Default values

  • Handled only once by the FieldFactory object. Later during lifecycle of the field the default value is not resolvable.
    • Consequence - during change of locale there is no straightforward way to set the default value for a localized property ( MGNLUI-3489 - Getting issue details... STATUS ).

Read-only state

Resolved only based on the setting coming from the field definition, should there be other factor that might influence the field's read-only state - they are omitted ( MGNLUI-3448 - Getting issue details... STATUS ).

  • AbstractFieldFactory#setConstraints - accesses field's property datasource to set read-only state, could be done within transformer object.
  • BasicTransformer#readFromItem() sets the read-only state of the underlying item property which does not make a lot of sense (TransformedProperty backing-up the field never reads this value).

First solution attempt: "reactive" form approach

The main idea of this solution is that when major changes happen (e.g. locale switched) - the form should just be re-generated instead of trying to adapt to the changes by manually modifying the underlying structures. Along the way we try to stream-line the way the read-only state and default field values are handled.

Why this approach should work? When we re-generate the form - we only re-create the UI part and the property Transformers, the underlying data (e.g. JCRNodeAdapter) would stay modified and intact, i.e. we re-bind the newly generated fields to the same Vaadin item. That item is anyway has to fully represent the changed state of the data (required by actions such as SaveDialogAction which simply call JcrNodeAdapter#applyChanges() method).

Steps to make

Allow detail sub-app/form dialog to re-generate the form. Can be done fairly easily without major API changes:

  • FormView should become clearable - i.e. all fields should be easily removed.
  • FormBuilder#buildForm() once again.

Use authoring locale when generating fields. Since we now track the current authoring locale at least in SubAppContext (would probably make sense to propagate it to whole UIContext) - that is also not hard to implement:

  • Inject SubAppContext/UiContext into FieldFactories, set authoring locale to property transformers and fields themselves. 

Refactor transformers and TransformedProperty. The most sensitive part (esp. with multi/composite transformers):

  • Transfomers should now assume they work with just one locale - no need to be able to track several locales all at once.
  • Simplify i18n support:
    • BasicTransformer#i18NPropertyName becomes unnecessary - we don't need to set it externally, we can inject I18NAuthroingSupport and generate i18n-aware property name via it internally.

    • TransformedProperty's i18n-related methods (hasI18NSupport and fireI18nValueChange) become also redundant since TransformedProperties will be regenerated.

    • For read-only state support: add read-only state setter/getter to Transformer interface and make TransformedProperty delegate to it:

      @Override
      public boolean isReadOnly() {
      return super.isReadOnly() && transformer.isReadOnly();
      }
    • Delegating transformers should adapt to one locale case:

      • i.e. DelegatingMultiValueFieldTransformer#items which tracks separate items for all locales - should be replaced with a single item for the current locale.
      • TBD: research and prove with PoC.

Other consequences:

  • DefaultI18NAuthoringSupport#i18nize() wouldn't be needed any longer - all the ugliness could go away.

  • Default values and read-only states would be much better and logically respected during locale changes since FieldFactories would re-apply those on form re-generation.

Effort source:

https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=shortlog;h=refs/heads/refactor/MGNLUI-3490


 

 

 

  • No labels