-
Notifications
You must be signed in to change notification settings - Fork 205
Add controllers doc #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add controllers doc #134
Conversation
} | ||
``` | ||
|
||
The controller instance registers itself to receive lifecycle callbacks from the component. The component associated with a controller instance is called the host component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move this paragraph after the example and explain that the controller can trigger an update and that in this example, the updated time will be rendered periodically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the bit about triggering updates, but I'm inclined to keep the detail here instead of with the new example I added above, to keep the intro high-level.
|
||
Reactive controllers can be implemented with JavaScript classes. | ||
|
||
A controller needs a reference to its host component so that it can register itself and interact with the component later: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A controller needs a reference to its host component so that it can register itself and interact with the component later: | |
A controller registers itself with its host component by calling `addController`. Typically, it stores a reference to its host component so it interact with the host. For example, to provoke an update on the host, the controller can call `requestUpdate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be an interactive example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added example to the top of the page
|
||
{% endtodo %} | ||
|
||
#### Controller that own Directives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#### Controller that own Directives | |
#### Controllers that own directives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. All my comments are nits. Would like to see them addressed but happy to have this merged.
@@ -0,0 +1,181 @@ | |||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this whole thing included? Is it because it's not published? Can we at least make a TODO to remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we publish with my latest changes we can remove this.
packages/lit-dev-content/site/_includes/projects/docs/controllers/overview/clock-controller.ts
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
const timeFormat = new Intl.DateTimeFormat('en-US', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make the controller do this?
|
||
{% endtodo %} | ||
|
||
## See Also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike having a See also
section at the end. It's out of context and likely the user that finally arrives here wanted this info long ago. Feel like these links should be integrated into the content where they make sense.
@arthurevans PTAL |
You can use controllers to implement features that require their own state and access to the component's lifecycle, such as: | ||
|
||
* Handling global events like mouse events | ||
* Managing asynchronous tasks like fetching data over the network | ||
* Running animations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid that to a new reader, the top examples in the bullets don't obviously connect with "features that require their own state and access to lifecycle".
Handling events and fetching data themselves don't really require state or hooking element lifecycle. It's allowing the owner to act on the results of them that requires triggering updates. Maybe it would connect better to include "triggering updates" in the description somehow, like
You can use controllers to implement features that require their own state and access to the component's lifecycle, such as: | |
* Handling global events like mouse events | |
* Managing asynchronous tasks like fetching data over the network | |
* Running animations | |
You can use controllers to implement features that require their own state and access to the component's lifecycle, such as: | |
* Triggering component updates based on global event listeners like mouse events | |
* Managing asynchronous tasks like fetching data over the network and triggering updates when tasks complete | |
* Running animations |
...or something (not sure that's great). But just saying "handling events" doesn't make the connection I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2 would be simpler in the singular, like "Managing an asynchronous task like fetching data over the network, and triggering an update when the task completes."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point in (1) is not triggering the update necessarily - you can already do that with el.requestUpdate()
- but using the hostConnected
and hostDisconnected
callbacks to add and remove the listeners.
Similarly with tasks, a key feature of controller is having the hostUpdate
hook so you can pull new deps.
|
||
You can also create controllers that are specific to `HTMLElement`, `ReactiveElement`, `LitElement` and require more of those APIs; or even controllers that are tied to a specific element class or other interface. | ||
|
||
`LitElement` and `ReactiveElement` are controller hosts, but hosts can also be other objects like base classes from other web components libraries, components from frameworks, or other controllers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`LitElement` and `ReactiveElement` are controller hosts, but hosts can also be other objects like base classes from other web components libraries, components from frameworks, or other controllers. | |
`LitElement` and `ReactiveElement` are controller hosts, but hosts can be any object that implements the [ReactiveControllerHost](/guide/api/controllers/#ReactiveControllerHost) interface described above; these could include base classes from other web components libraries, components from frameworks, or other controllers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like name & link should occur up above, rather than down here?
packages/lit-dev-content/samples/docs/controllers/forex/forex-controller.ts
Outdated
Show resolved
Hide resolved
} | ||
|
||
constructor(host: ReactiveControllerHost) { | ||
(this.host = host).addController(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For understanding, the addController
here isn't really required right? This is more of a "controller wrapper" than "controller proper", since it doesn't implement any callbacks itself. Not sure that makes it a bad example per se, but having this addController
here sort of makes it unclear what's going on since it's not actually required and wouldn't need the implements ReactiveController
.
But it begs the question, Is a controller a controller if it only calls requestUpdate
and doesn't implement any of the ReactiveController
interface...? Seems like by this example's definition, a controller is either an object that implements ReactiveController
interface, or else somehow gets and doinks with a ReactiveControllerHost
, or both.
Sorta makes a controller harder to define.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be clearer if you removed the addController
here and added a comment that this is basically a wrapper that configures the task controller and exposes a render()
method to the host component...
Or it could act as a controller, and delegate the callbacks to task
. Probably less efficient that way, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the addController() call, but yeah, controller are a bit hard to defined because they're helpers and can use any amount of API on the host. If we try to limit the use of "controller" to object passed to addController()
we're going to have to figure out what to call these other things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments, basically looking good. Need to decide whether we're finishing the sections on controller directives & animations for MVP.
Tasks example feels weird and complicated. Maybe it just needs a little more explanation or comments on the main controller?
You can use controllers to implement features that require their own state and access to the component's lifecycle, such as: | ||
|
||
* Handling global events like mouse events | ||
* Managing asynchronous tasks like fetching data over the network | ||
* Running animations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2 would be simpler in the singular, like "Managing an asynchronous task like fetching data over the network, and triggering an update when the task completes."
|
||
You can also create controllers that are specific to `HTMLElement`, `ReactiveElement`, `LitElement` and require more of those APIs; or even controllers that are tied to a specific element class or other interface. | ||
|
||
`LitElement` and `ReactiveElement` are controller hosts, but hosts can also be other objects like base classes from other web components libraries, components from frameworks, or other controllers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like name & link should occur up above, rather than down here?
|
||
`@lit-labs/task` contains a generic `Task` controller that can pull inputs from the host, execute a task function, and render different templates depending on the task state. | ||
|
||
You can use `Task` to create a custom controller with an API tailored for your specific task. Here we wrap `Task` in a `ForexController` that can fetch foreign exchange data from a REST API. `ForexController` exposes a `currency` property as an input, and a `render()` method that can render one of four templates depending on the task state. The task logic, and how it updates the host, are abstracted from the host component. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this example too complicated? I had trouble following it, and the main controller, as Kevin noted, it's not obvious why it needs to be a controller. Also the task.ts has a ton of "TODO" type comments in it, which feels a little weird in an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have task.ts updated in @lit-labs/task
. Will TODO to switch to that and remove it from the demo
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
Co-authored-by: Arthur Evans <arthure@google.com>
No description provided.