-
Notifications
You must be signed in to change notification settings - Fork 39
Conversation
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.
Minor nits, grammar and typo fixes, and a few suggestions for improved clarity.
README.md
Outdated
|
||
The `Registry` provides a mechanism to define widgets and injectors (see the [`Containers & Injectors`](#containers--injectors) section), that can be dynamically/lazily loaded on request. Once the registry widget is loaded all widgets that need the newly loaded widget will be invalidated and re-rendered automatically. | ||
|
||
A main registry can be provided to the `projector`, which will be automatically passed to all widgets within the tree (referred to as `baseRegistry`). Each widget also gets a access to a private `Registry` instance that can be used to define widget specific registry items. The locally defined registry items are considered a higher precedence than an item registered in the `baseRegistry`. |
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.
gets a access -> gets access
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.
widget specific registry -> widget-specific registry
README.md
Outdated
projector.setProperties({ registry }); | ||
``` | ||
|
||
In some scenarios, it might be desirable to allow the `baseRegistry` override the an item defined in the local `registry`. To do this use pass `true` as the second argument of the `registry.get` function. |
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.
baseRegistry
override the an item -> baseRegistry
to override an item
README.md
Outdated
projector.setProperties({ registry }); | ||
``` | ||
|
||
In some scenarios, it might be desirable to allow the `baseRegistry` override the an item defined in the local `registry`. To do this use pass `true` as the second argument of the `registry.get` function. |
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.
To do this use pass true
as the second argument of the registry.get
function. -> Use true
as the second argument of the registry.get
function to override the local item.
README.md
Outdated
|
||
In some scenarios, it might be desirable to allow the `baseRegistry` override the an item defined in the local `registry`. To do this use pass `true` as the second argument of the `registry.get` function. | ||
|
||
The following example, sets a default loading indicator widget but passes `true` when getting the widget from the `registry`. This allows a consumer to register an overriding loading indicator in the `baseRegistry`. |
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.
example, sets a default loading indicator widget but -> example sets a default loading indicator widget, but
README.md
Outdated
|
||
TODO: Section | ||
Occasionally, in a mixin or a widget class, it my be required to provide logic that needs to be executed before properties are diffed (`beforeProperties`) or either side `render` call (`beforeRender` & `afterRender`). This functionality is provided by the `beforeProperties`, `beforeRender` and `afterRender` decorators that can be found in the `decorators` directory. |
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.
"or either side render
call" feels like a word is missing
README.md
Outdated
} | ||
} | ||
``` | ||
|
||
**Please Note:** Don't use container as the top level application widget, containers always `render` which means that you'll find your application is calling `render` every request animation frame even if there is no need for the `render`. |
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.
Don't -> Do not
you'll -> you will
calling render
every request animation frame even if there is no need for the render
. -> calling render
upon every request animation frame, even when there is no need for an additional render
.
README.md
Outdated
|
||
### Decorators | ||
|
||
All built-in decorators can be used in non-decorator environments (Either JavaScript/ES6 or a TypeScript project that does not have the experimental decorators configuration set to true in the `tsconfig`) programmatically calling them directly, usually within a Widget classes `constructor`. |
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.
we use builtin, built in, and built-in, we should decide on one and be consistent
README.md
Outdated
|
||
### Decorators | ||
|
||
All built-in decorators can be used in non-decorator environments (Either JavaScript/ES6 or a TypeScript project that does not have the experimental decorators configuration set to true in the `tsconfig`) programmatically calling them directly, usually within a Widget classes `constructor`. |
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.
programmatically calling -> programmatically by calling
README.md
Outdated
|
||
### Decorators | ||
|
||
All built-in decorators can be used in non-decorator environments (Either JavaScript/ES6 or a TypeScript project that does not have the experimental decorators configuration set to true in the `tsconfig`) programmatically calling them directly, usually within a Widget classes `constructor`. |
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.
Widget classes -> either Widget class' or Widget class ?
README.md
Outdated
@@ -619,7 +710,7 @@ The currently supported options: | |||
|-|-| | |||
|`onAttached`|A callback that is called when the wrapped DOM is flowed into the virtual DOM| | |||
|
|||
For example, if we want to integrate a 3rd party library where we need to pass the component factory a _root_ element and then flow that into our virtual DOM. In this situation we don't want to create the component until the widget is being flowed into the DOM, so `onAttached` is used to perform the creation of the component: | |||
For example, if we want to integrate a 3rd party library where we need to pass the component factory a _root_ element and then flow that into our virtual DOM. In this situation we do not want to create the component until the widget is being flowed into the DOM, so `onAttached` is used to perform the creation of the 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.
3rd party -> third-party
Thanks for the review @dylans feedback addressed (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.
one typo, after that this is good to merge
README.md
Outdated
@@ -535,7 +535,7 @@ class MyWidget extends WidgetBase { | |||
|
|||
The `Registry` provides a mechanism to define widgets and injectors (see the [`Containers & Injectors`](#containers--injectors) section), that can be dynamically/lazily loaded on request. Once the registry widget is loaded all widgets that need the newly loaded widget will be invalidated and re-rendered automatically. | |||
|
|||
A main registry can be provided to the `projector`, which will be automatically passed to all widgets within the tree (referred to as `baseRegistry`). Each widget also gets a access to a private `Registry` instance that can be used to define widget specific registry items. The locally defined registry items are considered a higher precedence than an item registered in the `baseRegistry`. | |||
A main registry can be provided to the `projector`, which will be automatically passed to all widgets within the tree (referred to as `baseRegistry`). Each widget also gets access to a private `Registry` instance that can be used to define registry items the are scoped to the widget. The locally defined registry items are considered a higher precedence than an item registered in the `baseRegistry`. |
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.
the are scoped -> that are scoped
Type: bug
The following has been addressed in the PR:
Description:
Add sections for
beforeProperties
Registry
Container/Injectors
Resolves #669
Resolves #671