Commit 27d9079d authored by Robert Knight's avatar Robert Knight

Update the ADR in response to CR feedback

 * Separate the 'Context' section into relevant background information
   and client-specific context.

 * Explicitly state in the 'Decision' section that `angular.component()`
   will be used.
parent e219757a
......@@ -4,6 +4,8 @@ ADR 1: Component architecture for sidebar application
Context
-------
## Background
Historically front-end web frameworks, including Angular 1.x, used a variety of
MVC-based patterns for structuring the user interface part of an application
, which of course is much of the code for a web app.
......@@ -30,23 +32,40 @@ simpler framework implementations.
In Angular JS prior to v1.5, this pattern could be achieved by using a
combination of features (element directives, isolate scope, bind-to-controller,
controllerAs) and all of our new UI components written during 2016 followed this
pattern. However, it isn't that obvious until you know the codebase well that
this is the way the UI is structured.
Angular JS 1.5x introduced [explicit
controllerAs). Angular JS 1.5x introduced [explicit
support](https://docs.angularjs.org/guide/component) for this architecture via
`.component()`.
## Components in the client
The client historically used traditional Angular 1.x methods of passing data
between parts of the UI - properties on the scope inherited by child scopes and
events. As the app grew larger it became harder to reason about where data in
templates came from and who could change it ("$scope soup"). Newer parts of the
UI have used element directives with isolate scopes to implement a
component-based architecture, thus avoiding this problem. However:
- This requires a bunch of boilerplate for each directive.
- It isn't clear that we use this pattern from looking at the entry point to the
sidebar.
- Important parts of the top level of the UI (the `*-controller.js` files and
`viewer.html`) do not use this pattern and suffer from being hard to
understand in isolation. Additionally they use a different mechanism
(`ngRouter`) to control what is displayed than the rest of the UI, where we use
`ng-if` guards.
- This lack of consistency makes it difficult to understand how the top level of
the UI works.
Decision
--------
We will convert all element directives in `src/sidebar/directive` to components
and move them to `src/sidebar/components`. This change will be simple in most
cases and will require some moderate refactoring for others.
* We will convert all element directives in `src/sidebar/directive` to
components (ie. change them to use `angular.component()` and any refactoring
this implies) and move them to `src/sidebar/components`. This change will be
simple in most cases and will require some moderate refactoring for others.
The top-level of the UI which consists of a mix of router, controller
and templates unlike the rest of the UI will also be converted to this pattern.
* The top-level of the application will also be converted to a set of components
using the same pattern and the router will be removed.
Status
------
......@@ -56,11 +75,8 @@ In discussion
Consequences
------------
In the short term, this change should improve the consistency of how UI
components are defined and how they communicate with one another. It should also
make it easier to understand and refactor parts of the client in isolation. It
is hoped this will also make it easier for newcomers to understand how the UI is
structured and reduce the number of concepts that they need to grok.
In the short term this should make it easier to understand the sidebar app by
improving consistency.
In the medium term, this brings the architecture of the client more into
alignment with how it would be structured in other frameworks and gives us the
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment