Kibana: Document Component Guidelines

[w33ble]

This was sparked by this comment on the UI cleanup issue.

We have started to write reusable components, using Angular directives and in some case React components. Many of us have some general ideas about what makes a good, reusable component, but it's important that we document them and create a reference for the whole team.

This issue is to track the creation of a component styleguide/reference. Where that lives and how it gets implemented is pretty much at the discretion of the person that does it.

[cjcenizal]

This is borrowed from the email I sent out recently:

Examples of components

• Some of the table directives in the Watcher UI
• The LoadingButton and SizePicker directives in Felix's event context PR

Angular components

Based on these examples, I can sort of see where we're going in terms of componentized directive design, because they all seem to exhibit these traits:

1. They're relatively logic-free. Logic is provided via the attributes, in the form of callbacks and "providers" (think of a list or table directive accepting a "sort" function which provides the directive with sorting logic).
2. They typically use ng-transclude to place child elements inside of an element in the template. From this perspective, they simply wrap children with specific markup.
3. They're as small and simple as possible, sometimes consisting of a single element with a class or two on it.
4. If a directive can enter various states (e.g. a button can enter binary states of disabled or enabled, or the multivariate states of "primary", "basic", and "danger"), then you can pass boolean flags or enum values to it via its attributes to set it to the desired state(s).

[w33ble]

One of the things that your list of traits touches on, but doesn't explicitly say, is that their state should be pushed up as high as possible. That is, most reusable components should have no state at all, with all of their "state" coming in as attributes/props, and their consumer should be in charge of passing that in.

Also implied here, user interactions should be handled by passed in handlers, which trigger actions in the consumer, instead of mutating state directly.

[cjcenizal]

I should also mention that I think using replace: true is a best practice, since markup structure and CSS can sometimes have some level of coupling (especially when flexbox is involved, since flexbox is all about the parent-child relationship).

[Bargs]

An interesting issue came up during the event context review with @weltenwort. Because our version of Angular lacks one way binding it's difficult to separate local state (which is fine in small doses, IMO) from global state (which is evil).

The discussion came up in the context of a new size-picker component. size-picker accepts a count param, which it binds to an input element with ng-model. Under normal conditions, changes to the input element would propagate all the way up to whatever variable is two-way bound to count in the parent component, which is undesirable. @weltenwort came up with an interesting workaround using a getterSetter with ng-model. Instead of setting count directly, it would invoke a callback and rely on the parent component to update the state appropriately.

The problem is that if an error occurs in the callback the component will be left in an invalid state, with the input value set to the user entered value while the local $scope variable still holds the old value.

I suggested an alternative solution using a couple of watches to implement a poor man's one-way binding, but that wasn't without its own issues, it created a choppy user experience when @weltenwort tried it.

So, that's the (long winded) description of the problem. It would be cool if we could come up with a standard way to solve it. Does anyone else have ideas for other patterns that might work? Or what's keeping us from upgrading Angular?

[w33ble]

I came across this problem recently as well. My solution was to copy the value in the controller's constructor, and use my copied value with ng-model.

app.directive('mySelect', function () {
  return {
    restrict: 'E',
    replace: true,
    template: `
      <select 
        ng-model="mySelect.value" 
        ng-change="mySelect.onValueChange(mySelect.value)"
      >
        <option value="1">1</option>
        <option value="2">2</option>
        <option value="3">3</option>
        <option value="4">4</option>
      </select>
    `,
    scope: {
      passedValue: '=value',
      onValueChange: '=',
    },
    controllerAs: 'mySelect',
    bindToController: true,
    controller: class mySelectController {
      constructor() {
        // avoid parent state mutation, since we have no one-way binding
        this.value = this.passedValue;
      }
    }
  };
});

It's not pretty, but it works, and it's quick and easy with primitive values. Objects and arrays would need to be deep cloned of course.

As for why we haven't updated Angular, I know someone looked at it a while back, but it didn't get completed. @spalger is this still on your radar?

[Bargs]

@w33ble how do you propagate updates from passedValue to this.value if it changes after construction?

[w33ble]

Good question. All I can say is that this worked correctly where I've done it already.

Maybe this needs a $watch on passedValue though? I honestly don't recall if you need a $watch on passed in values or not, I have a habit of thinking in React/Vue, and I don't use Angular for a long enough stretch remember 😞 .

[cjcenizal]

Closing this as we’re moving to React and there’s a ton of material out there on best practices around component design.