Patterns for extending/customizing widgets

Design

We need to provide better patterns/direction on extending or customising the behaviour of widgets.

This also covers the more macro topic of how we extend behaviour as a whole, but especially with our out of the box widgets, we should give clear guidance on if they are extensible, or if they should be reimplemented and we focus on exploding out functionality to make it easy to re-implement a widget without much effort.

As specific use case that I encountered was in wanting a tab pane, where changing tabs updated a 3rd party container versus changing the widget being displayed (e.g. the web-editor and the monaco Editor). Instead of being able to access the tab bar as a discrete widget, I had to essentially reimplement the whole TabPane widget. We need to go through a design process on this.

There are already a couple issues that relate to this, which I will organise into an Epic.

We also need to consider how we extend styles and themes as they relate to widgets. How we expect people to leverage existing styles and themes built around those styles, or again, if it is a 100% reimplementation.

Some thoughts on component modification.

TLDR; inheritance feels right.

As a consumer of Dojo 2 widgets I should be able to modify default behavior in a uniform manner to meet the needs of my application.

Using `properties`

The most obvious form of customization comes through the use of a component's public API exposed through its properties interface:

w(TabController, {
  alignButtons: Align.right
})

Passing different values for specific properties, like passing Align.right for alignButton above, allows for efficient behavioral modification of component instances. For example, if a TabController's tabs are normally left-aligned in an application but are right-aligned in one specific use case, passing in a value for alignButtons is an optimal solution for one-off customization.

Though using properties works well for instance-based modification, this practice can be both restrictive and tedious in common application scenarios. For example, if a TabController's tabs should always be right-aligned throughout an application, alignButtons would need to be explicitly set to Align.right anytime the component is used, which is both brittle and unmaintainable:

w(TabController, {
  alignButtons: Align.right,
  tabs: tabsA
}),
w(TabController, {
  alignButtons: Align.right,
  tabs: tabsB
}),
w(TabController, {
  alignButtons: Align.right,
  tabs: tabsC
})

Further, and perhaps more importantly than the verbosity above, modification through properties restricts what can and can't be changed for a given component to only the properties exposed by the widget author, something that may be problematic when providing a library of pre-fabricated widgets intended for wide situational reuse. Unless we explicitly account for the specific modification a downstream developer wishes to accomplish, it's just not possible using properties.

Using higher order components

A common solution to sharing code between components and also to solving the verbosity and maintainability issues above is through the use of higher order components. Using this pattern, a function is called that returns a new component that wraps an underlying component that can have conditional behavior or rendering based on function arguments. For example, continuing with the TabController example above, if a TabController's tabs should always be right-aligned throughout an application, a higher order component could be used to abstract away the alignButtons property:

v('div', { key: 'some-tabs' }, [
  w(createTabs(tabsA))
]),
v('div', { key: 'some-more-tabs' }, [
  w(createTabs(tabsB))
]),
v('div', { key: 'some-other-tabs' }, [
  w(createTabs(tabsC))
])

The only parameter for this example function is tabs so that different tab arrays can be passed into each wrapped TabController, but any number of parameters could be used:

function createTabs(tabs) {
  return class extends WidgetBase {
    protected render(): DNode {
      return w(TabController, {
        alignButtons: Align.right,
        tabs
      });
    }
  };
}

This pattern has several advantages. It allows for the modification of components through properties without the lack of maintainability caused by explicitly passing in the same property values every time a component is used, as seen above. This pattern also effectively allows for shared logic between components by returning a wrapped component that can contain common code; the generating function can accept a component class as a parameter for greater internal rendering decoupling:

function createAutoSaveComponent(Component) {
  return class extends WidgetBase {
    save() {
      // common save logic
    }

    protected render(): DNode {
      return w(Component, {
        onClose: save
      });
    }
  };
}

Despite the flexibility and maintainability advantages that higher order components can provide, they still inherently rely on properties to customize the underlying component that's wrapped. Again, this could be a drawback in the context of internal use within a library of components: because inheritance isn't used to copy behavior from the underlying component, the same property-based restrictions as to what downstream developers can and can't modify still apply even within higher order components.

Using inheritance

ES6 class-based inheritance provides another mechanism to customize the behavior and UI of a component in a reusable manner. Because Dojo 2 components are already authored using inheritance by way of WidgetBase and other base classes, the pattern of extending a component class and overriding members is a familiar one for downstream developers. Say an application had a requirement to use native HTML radio inputs instead of default Dojo 2 TabButtons to control tabs:

class MyTabController extends TabController {
  renderTabButtons() {
    return this._tabs.map((tab, i) => {
      return w('Radio', {
        // ...
      })
    };
  }
}

renderTabButtons could be overridden to provide a modified DNode[] and static type checking enforces that the overridden method signature matches.

Popular functional frameworks like React have warned against using inheritance in certain situations for a key reasons. Extending many classes can introduce layers of chained complexity that can make it difficult to determine where existing functionality lives or new functionality should be added. This point isn't specific to functional programming and is heavily backed up by Dojo 1's spider web architecture of circular mixins and base classes. Second, React warns that inheritance introduces implicit, tightly-coupled code dependencies, such as a base class calling a method in the parent class or vice versa. React implies that this tight coupling between base and parent class is brittle and leads to unexpected naming errors when changing member names, which isn't maintainable.

In the context of Dojo 2 widgets written in TypeScript, the maintainability argument isn't applicable; intellisense and tsc will indicate that name clashes or errors exist when downstream developers change component code that extends our default components. This is a powerful advantage of using static type checking. TypeScript goes further and allows our components to strictly adhere to method visibility patterns, only exposing methods intended to be overridden using the protected keyword, giving downstream developers clear, self-documented extension points.

The type of modification provided by inheritance, even inheritance controlled by member visibility keywords, does come with special considerations. In Dojo 2, it's almost never correct to extend the render method of a pre-fabricated component. As such, most of a component's render method should off-load element generation to helper methods:

protected renderResult(result: any): DNode {
  const { getResultLabel } = this.properties;
  return v('div', [ getResultLabel(result) ]);
}

render(): DNode {
  const {
    isDisabled,
    result,
    selected
  } = this.properties;

  return v('div', {
    'aria-selected': selected ? 'true' : 'false',
    'aria-disabled': isDisabled(result) ? 'true' : 'false',
    classes: this.classes(
      css.option,
      selected ? css.selected : null,
      isDisabled(result) ? css.disabledOption : null
    ),
    'data-selected': selected ? 'true' : 'false',
    onmousedown: this._onMouseDown,
    onmouseenter: this._onMouseEnter,
    onmouseup: this._onMouseUp,
    role: 'option'
  }, [
    this.renderResult(result)
  ]);
}

This example code is taken from ResultItem for ComboBox; renderResult would be extended, shielding downstream developers from having to reimplement the mostly-internal-specific render method. This pattern of off-loading element generation logic to helper methods allows for safer component extension using simpler method signatures.

Using the registry

The Dojo 2 widget system provides the concept of a registry that can be used to swap out classes used internally by a component. For example, if the TabController made use of an internal registry, it may accept a customTabButton property that could be any class that implements the TabButton interface. A registry is a powerful tool for providing entire components to another component for internal use, a form of component modification that allows entire portions of a component to be modified at once. Because most uses of the registry involve providing a component class that extends some default component class, inheritance again feels natural:

class CustomTabButton extends TabButton {
  renderButton(result: any) {
    return w('Radio', {
      // ...
    });
  }
}

w(TabController, {
  customTabButton: CustomTabButton,
  // ...
}),

Conclusion

After formalizing these thoughts and writing example code, and because higher order components still rely on properties, the following solutions exist for component modification:

properties / HOC
Inheritance

I feel that suggesting that downstream developers rely only on properties in the context of a reusable enterprise framework like Dojo 2 is a mistake. I worry that this prevents behavioral modification by way of overly-locking down what can and can't be modified.

Using proper member visibility, off-loaded render helpers, and supporting registry-swapped internal components is the most effective and natural form of customization in Dojo 2, and one that embraces the language features provided by TypeScript.

Great write-up @bitpshr. 👍 for inheritance.

I'm on board the inheritance 🚂. Agreed, good writeup, @bitpshr.

Assuming we're taking that approach, I have just a few thoughts on patterns we should follow to make widgets easily extensible:

(most obviously) break everything up into smaller functions, especially render
if the widget uses this.children, always abstract to something like a protected getContent() function
State classes especially (e.g. .invalid, .open, etc) should be returned in a separate fn
Icon VNodes, or the immediate vdom containing icons (e.g. previous/next buttons in calendar) should have their own function
Any inline styles (e.g. thumb position in Slider) should be separate

Anything else? Those are what I've run across recently when trying to customize specific widgets, but I'm sure there's more.

One other change I think we should make is to allow properties to be extended along with widgets, so instead of writing e.g. export default class Button extends ButtonBase<ButtonProperties> we would have export default class Button<P extends ButtonProperties = ButtonProperties> extends ButtonBase<P>.

Per chatting with @agubler, I've drafted some examples where extending properties would be necessary for a few widgets. The examples all assume our existing widgets have been modified to include more overridable hooks.

Icon Button

export interface IconButtonProperties extends ButtonProperties {
  icon: string;
}

@theme(css)
export default class IconButton extends Button<IconButtonProperties> {
  getContent() {
    return [
      v('i', {
        classes: this.classes(css.iconAddon, iconCss.icon, iconCss[this.properties.icon]),
        role: 'presentation',
        'aria-hidden': 'true'
      }),
      ...this.children
    ];
  }
}

TitlePane with thumbnail

export interface BlogAccordionProperties extends TitlePaneProperties {
  titleImage: string;
  titleImageDescription: string;
}

@theme(css)
export default class BlogAccordion extends TitlePane<BlogAccordionProperties> {
  renderTitle() {
    const { title, titleImage, titleImageDescription } = this.properties;
    return [
      v('img', {
        alt: titleImageDescription,
        classes: this.classes(css.thumbnail),
        src: titleImage
      }),
      title
    ];
  }
}

Masked Input

export interface MaskedInputProperties extends TextInputProperties {
  mask: string;
  maskDefinitions: { [key: string]: string };
}

@theme(css)
export default class MaskedInput extends TextInput<MaskedInputProperties> {
  onChange(event: KeyboardEvent) {
    const value = this.getPlainValue(event.target.value);
    const { key, onChange } = this.properties;
    onChange && onChange(value, key);
  }

  getMaskedValue(value: string) {
    const { mask, maskDefinitions } = this.properties;
    // mask logic
    return maskedValue;
  }

  getPlainValue(value: string) {
    const { mask, maskDefinitions } = this.properties;
    // unmask logic
    return plainValue;
  }

  getValue() {
    return this.getMaskedValue(this.properties.value);
  }
}

@nicknisi ran across this issue today, and I think it's worth noting:

Any widget that internally uses child widgets should allow them to be customized through the widget registry. Currently, it wouldn't be possible to extend and make changes to TabButton when using TabController, for example.

Maybe we should also consider passing down extraClasses to child widgets, similarly to needing to pass down theme. If all components share the same css module file (as has been the case so far) there wouldn't be any conflicts, and I think it would be most intuitive for the end user to be able to override all classes across the entire component.

Overall summary of Things To Do™ for widgets:

Changes needed within the existing widgets

Allow individual widget properties to be extended when extending the widget, so instead of: export default class Button extends ButtonBase<ButtonProperties> we would have: export default class Button<P extends ButtonProperties = ButtonProperties> extends ButtonBase<P>
Break out render() code into more manageable chunks, specifically in the following ways:
- For widgets that generate more DOM, move sensible chunks into separate functions, e.g. renderControls() for the buttons in ComboBox, or renderOptions in Listbox.
- For widgets that render children, e.g. TabController, return this.children in a separate function. Just as an example, this would make it much easier to add custom loading content.
- Split "state" classes into a separate function (getModifierClasses?) for all widgets that could potentially support multiple visual states, e.g. loading or dirty on a tab button.
- If a widget uses an icon, generate the icon separately. Swapping out icons or using something other than a font icon for a visual symbol seems like a fairly common modification, e.g. changing left/right month arrows in Calendar for text, or tiny month preview images.
- Widgets that calculate inline styles (right now only Slider, I think) should do so in a separate fn. This would allow a triangular volume slider to adjust the height of the thumb as it changes, for example.
Update widget events to not return the event object. Instead, we should return whatever value is appropriate to the function (e.g. index for onRequestTabChange in TabController), along with the optional widget key for all event handlers. Always passing back the widget key allows users to have a single handler for multiple widgets with individual widget logic. For example, a form widget with multiple form fields could have a single onInput handler for all of them:

_input1 = '';
_input2 = '';
_input3 = '';

private _onInput(value: string, key: string | number) {
  this[`_${key}`] = value;
  this.invalidate();
}

render() {
  return v('div', [
    w(TextInput, {
      key: 'input1',
      onInput: this._onInput
    }),
    w(TextInput, {
      key: 'input2',
      onInput: this._onInput
    }),
    w(TextInput, {
      key: 'input3',
      onInput: this._onInput
    })
  ]);
}

Some sort of generated report of which module each className comes from, or any other tool to aid in debugging where styles come from

Additional documentation

When to re-theme vs. when to add properties that control styling classes. Right now my approach would be to ask "would I normally handle this with multiple classes (i.e. button large primary), or with an entirely separate class (i.e. slider vs. volume-control)?" Another way to phrase it is "Am I creating an entirely new visual style, or am I merely tweaking a few style properties?"
More writing around how to trace where specific styles are coming from / debugging themes (related to (4) above)

+1 for all of the above from @smhigley. Nice writeup.

One more Thing To Do™:

To preserve one-widget-one-css-module, we should no longer use sub-widgets (e.g. SelectOption for Select) as a customization path. The existing widgets that do this (TabButton in TabController and Datepicker/CalendarCell in Calendar) should be modified. Additionally, we should probably give Tab its own css module, separate from the TabController css.

Update to the above ^

Per discussion with Ant, we're going to keep the sub-widgets around, but as internal classes that aren't exposed to the user. This maintains our current method of creating event handlers that have access to sub-widget-specific data (i.e. onClick on w(ListboxOption) can easily take the correct index and data as parameters). Since these are not intended to be separate, stand-alone widgets, I believe we can continue using the same css module file for widgets + sub-widgets without issues.

For widgets that do stand alone or are exposed to users (e.g. TabController / Tab), I think we should still consider separating their css.

All of the actionable issues above have been separately ticketed out, and my thoughts on extensions have been captured in a blog post.

dojo / widgets