Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Update README Content #701

Merged
merged 6 commits into from
Oct 3, 2017
Merged

Update README Content #701

merged 6 commits into from
Oct 3, 2017

Conversation

agubler
Copy link
Member

@agubler agubler commented Oct 2, 2017

Type: bug

The following has been addressed in the PR:

  • There is a related issue
  • All code matches the style guide
  • Unit or Functional tests are included in the PR

Description:

Add sections for

  • beforeProperties
  • Registry
  • Container/Injectors

Resolves #669
Resolves #671

@agubler agubler requested a review from dylans October 2, 2017 11:58
Copy link
Member

@dylans dylans left a 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`.
Copy link
Member

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

Copy link
Member

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.
Copy link
Member

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.
Copy link
Member

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`.
Copy link
Member

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.
Copy link
Member

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`.
Copy link
Member

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`.
Copy link
Member

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`.
Copy link
Member

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`.
Copy link
Member

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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3rd party -> third-party

@dylans dylans added this to the 2017.10 milestone Oct 2, 2017
@agubler
Copy link
Member Author

agubler commented Oct 2, 2017

Thanks for the review @dylans feedback addressed (i think)

Copy link
Member

@dylans dylans left a 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`.
Copy link
Member

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

@agubler agubler merged commit fa44745 into dojo:master Oct 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants