This repository was archived by the owner on Sep 5, 2024. It is now read-only.

docs(mdCompilerProvider): add respectPreAssignBindingsEnabled docs #11255

Merged

andrewseguin merged 1 commit into master from mdCompilerProviderDocs

Apr 25, 2018

Contributor

Splaktar commented Apr 24, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

The commit message follows our guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

The docs site is missing the details about $mdCompilerProvider.respectPreAssignBindingsEnabled. The feature was added as part of #10016 and PR #10726. These docs can only be found in the code comments or CHANGELOG currently. It needs to be documented on the main docs site as well.

Issue Number:
Fixes #11252.

What is the new behavior?

$mdCompilerProvider.respectPreAssignBindingsEnabled is documented on the new $mdCompilerProvider docs page under Services.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

N/A

Splaktar added type: docs g3: reported P3: important labels

Splaktar added this to the 1.1.9 milestone

Splaktar requested a review from mgol

April 24, 2018 22:13

googlebot added the cla: yes label

Splaktar added the needs: review label

Contributor Author

Splaktar commented Apr 24, 2018

@gkalpak can you please take a look at these docs changes to verify that they are still accurate with AngularJS 1.6 and the plans for AngularJS 1.7?

gkalpak reviewed

View reviewed changes

Member

gkalpak left a comment

A couple of minor comments/typos.
LGTM otherwise 👍

src/core/services/compiler/compiler.js Outdated

    
                 * @returns {*} current value if used as getter or itself (chaining) if used as setter

                 * @param {boolean=} respected update the `respectPreAssignBindingsEnabled` state if provided, otherwise just return

                 * the current Material `respectPreAssignBindingsEnabled` state.

                 * @returns {boolean|function} current value if used as getter or itself (chaining) if used as setter

Member

gkalpak Apr 24, 2018

Why function? It returns the MdCompilerProvider instance if used as setter.

src/core/services/compiler/compiler.js Outdated

                * @module material.core.compiler
                * @description
-               * The $mdCompiler service is an abstraction of AngularJS's compiler, that allows developers
-               * to easily compile an element with options like in a Directive Definition Object.
+               * The `$mdCompiler` is able to respect the AngularJS `$compiler.preAssignBindingsEnabled` state when using

Member

gkalpak Apr 24, 2018 •

edited

Loading

$compiler.preAssignBindingsEnabled --> $compileProvider.preAssignBindingsEnabled (here and elsewhere).

src/core/services/compiler/compiler.js Outdated

+               * Only dynamic construction of elements such as Dialogs, Panels, Toasts, BottomSheets, etc. may be affected.
+               * Invoking `$mdCompilerProvider.respectPreAssignBindingsEnabled(true)` will make **bindings** in
+               * AngularJS Material custom components like `$mdDialog`, `$mdPanel`, `$mdToast`, or `$mdBottomSheet`
+               * only available in the controller constructors.

Member

gkalpak Apr 24, 2018

I believe this is wrong. (As mentioned below, invoking $mdCompilerProvider.respectPreAssignBindingsEnabled(true) can result in different behaviors, depending on $compileProvider.preAssignBindingsEnabled() and AngularJS version.)

Contributor Author

Splaktar Apr 25, 2018

I agree, I've cleaned this up.

src/core/services/compiler/compiler.js Outdated

-               *     compileData.element; // Compiled DOM element
-               *     compileData.link(myScope); // Instantiate controller and link element to scope.
+               *   app.config(function($mdCompilerProvider) {
+               *    $mdCompilerProvider.respectPreAssignBindingsEnabled(true);

Member

gkalpak Apr 24, 2018

Insufficient indentation 😇

src/core/services/compiler/compiler.js Outdated

-               *     compileData.element // Content Element (same as above)
-               *     compileData.link // This does nothing when using a contentElement.
+               *   app.config(function($mdCompilerProvider) {
+               *    $mdCompilerProvider.respectPreAssignBindingsEnabled(false);

Member

gkalpak Apr 24, 2018

Insufficient indentation. Also, this does not guarrantee that bindings are pre-assigned.

Contributor Author

Splaktar Apr 25, 2018

I believe that when $mdCompilerProvider.respectPreAssignBindingsEnabled(false) is called, it should guarantee that the bindings of AngularJS Material interim components are pre-assigned in AngularJS 1.5 and 1.6. Is this not correct? The description on line 119-120 seems pretty clear that the disabled (false) option doesn't depend on AngularJS versions or settings.

src/core/services/compiler/compiler.js Outdated

-               *     compileData.link // This does nothing when using a contentElement.
-               *   });
+               * function MyController() {
+               *   // Locals from Angular Material are set yet. e.g myVar is true.

Member

gkalpak Apr 24, 2018

are set yet --> are set already (?)


          docs(mdCompilerProvider): add respectPreAssignBindingsEnabled docs

cdd2918

Fixes #11252.

Splaktar force-pushed the mdCompilerProviderDocs branch from 423a6db to cdd2918 Compare

April 25, 2018 04:30

Splaktar added pr: merge ready pr: merge safe and removed needs: review labels

Splaktar assigned andrewseguin

gkalpak reviewed

View reviewed changes

src/core/services/compiler/compiler.js

-               * to easily compile an element with options like in a Directive Definition Object.
+               * The `$mdCompiler` is able to respect the AngularJS `$compileProvider.preAssignBindingsEnabled` state when using
+               * AngularJS versions between 1.5.10 and 1.7.0.
+               * See the [AngularJS documentation for `$compile.preAssignBindingsEnabled`

Member

gkalpak Apr 25, 2018

$compile --> $compileProvider

Contributor Author

Splaktar Apr 25, 2018

Doh! Not sure how I missed those, thanks.

src/core/services/compiler/compiler.js

-               * > The compiler powers a lot of components inside of AngularJS Material.
-               * > Like the `$mdPanel` or `$mdDialog`.
+               * To enable/disable whether the controllers of dynamic AngularJS Material components
+               * (i.e. dialog, panel, toast, bottomsheet) respect the AngularJS `$compile.preAssignBindingsEnabled` flag,

Member

gkalpak Apr 25, 2018

$compile --> $compileProvider

src/core/services/compiler/compiler.js

-                 * ](https://code.angularjs.org/1.6.4/docs/api/ng/provider/$compileProvider#preAssignBindingsEnabled)
-                 * for more information.
+                 * Call this method to enable/disable whether Material-specific (dialog/panel/toast/bottomsheet) controllers respect the
+                 * AngularJS `$compile.preAssignBindingsEnabled` flag. Note that this doesn't affect directives/components created

Member

gkalpak Apr 25, 2018

$compile --> $compileProvider

mgol suggested changes

View reviewed changes

src/core/services/compiler/compiler.js

-               * The $mdCompiler service is an abstraction of AngularJS's compiler, that allows developers
-               * to easily compile an element with options like in a Directive Definition Object.
+               * The `$mdCompiler` is able to respect the AngularJS `$compileProvider.preAssignBindingsEnabled` state when using
+               * AngularJS versions between 1.5.10 and 1.7.0.

Member

mgol Apr 25, 2018

It sounds like it respects it for 1.7.0 as well. How about:

 * The `$mdCompiler` is able to respect the AngularJS `$compileProvider.preAssignBindingsEnabled` state when using
 * AngularJS versions from 1.5.10 until (but not including) 1.7.0.

?

Contributor Author

Splaktar Apr 25, 2018

$compileProvider.preAssignBindingsEnabled is removed from AngularJS in 1.7.0. Maybe that's your point? I'll try to refine it.

src/core/services/compiler/compiler.js

+               * These constitute the majority of AngularJS Material and user-created components.
+               * Only dynamic construction of elements such as Dialogs, Panels, Toasts, BottomSheets, etc. may be affected.
+               * Invoking `$mdCompilerProvider.respectPreAssignBindingsEnabled(true)` will effect **bindings** in
+               * AngularJS Material custom components like `$mdDialog`, `$mdPanel`, `$mdToast`, or `$mdBottomSheet`.

Member

mgol Apr 25, 2018

$mdDialog etc. are services, not components. They don't even show components but usually templates with controllers (it'd be nice to add support for components but that's a separate story). Maybe:

...**bindings** in controllers created by AngularJS Material's services like $mdDialog, $mdPanel, $mdToast, or $mdBottomSheet.

?

src/core/services/compiler/compiler.js

                *   });
                * </hljs>
                *
-               * Example with a content element
+               * Assign Bindings Before the Constructor

Member

mgol Apr 25, 2018

This is only true for AngularJS <1.6.0 or AngularJS <1.7.0 if $compileProvider.preAssignBindingsEnabled(true) is called. Shouldn't it just say the reverse of the upper example's "Respect the AngularJS Compiler Setting"?

Splaktar removed the pr: merge ready label

andrewseguin merged commit 55ad25c into master

Member

mgol commented Apr 25, 2018

@andrewseguin @Splaktar It seems none of my comments were addressed in the merged commit; do you plan followups?

Splaktar added a commit that referenced this pull request


          docs(mdCompilerProvider): refinements for clarity

Relates to #11255.

Splaktar added a commit that referenced this pull request


          docs(mdCompilerProvider): refinements for clarity

00558e6

Relates to #11255.

Splaktar mentioned this pull request

docs(mdCompilerProvider): refinements for clarity #11257

Merged

3 tasks

Contributor Author

Splaktar commented Apr 25, 2018

@mgol yep, sorry there was a miscommunication there. #11257 addresses the review feedback. Thank you very much for reviewing!

andrewseguin pushed a commit that referenced this pull request


          docs(mdCompilerProvider): refinements for clarity (#11257)

039d2fe

Relates to #11255.

Splaktar added a commit that referenced this pull request


          docs(mdCompilerProvider): add respectPreAssignBindingsEnabled docs (#…

5fff619

…11255)

Fixes #11252.

Splaktar added a commit that referenced this pull request


          docs(mdCompilerProvider): refinements for clarity (#11257)

08785d4

Relates to #11255.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes g3: reported P3: important type: docs