Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs: update module.ngdoc Hello World example #8673

Closed

Conversation

definitelynotsoftware
Copy link
Contributor

Helpful for people new to Angular to see the ng-app declaration in context with the expression example. This will help illustrate the "Important thing to notice" point which follows: "The reference to myApp module in . This is what bootstraps the app using your module."

Helpful for people new to Angular to see the ng-app declaration in context with the expression example. This will help illustrate the "Important thing to notice" point which follows: "The reference to myApp module in <html ng-app="myApp">. This is what bootstraps the app using your module."
@@ -28,9 +28,11 @@ I'm in a hurry. How do I get a Hello World module working?

<example module='myApp'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add ng-app-included to the <example> tag for this to work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, I must be doing something wrong. I updated the commit as suggested but TravisCI is failing..getting this error: Error: No element found using locator: by.binding("{{ 'World' | greet }}")

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (CLA) signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let us know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@definitelynotsoftware
Copy link
Contributor Author

all set, CLA now signed

fixed indentation, replaced <html> w/ <div>, and added "ng-app-included" to <example>
@mary-poppins
Copy link

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@definitelynotsoftware definitelynotsoftware changed the title Update module.ngdoc Hello World example docs: update module.ngdoc Hello World example Aug 19, 2014
@caitp
Copy link
Contributor

caitp commented Aug 19, 2014

yeah we probably need to update the E2E test too, i'll help you with that in a sec

@caitp
Copy link
Contributor

caitp commented Aug 19, 2014

So, unfortunately we have an issue where the e2e tests are always going to try to use the "body" tag as the root element, and when we have ng-app-included="true", we aren't putting ng-app on the "body".

So, we can't really land this unless we get rid of those tests. Personally I don't think those are adding much so it might be good to get rid of some, but we'll see what other people think I guess

caitp added a commit to caitp/angular.js that referenced this pull request Aug 19, 2014
caitp added a commit that referenced this pull request Aug 21, 2014
caitp added a commit that referenced this pull request Aug 21, 2014
@@ -26,11 +26,13 @@ should be bootstrapped. There are several advantages to this approach:

I'm in a hurry. How do I get a Hello World module working?

<example module='myApp'>
<example ng-app-included module='myApp'>
Copy link
Contributor

Choose a reason for hiding this comment

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

I've landed a fix for the e2e bugs you were getting. So can I get you to change this very slightly, to make it ng-app-included="true" instead? We need a value, otherwise the template engine doesn't know that there's a value for it.

Also fix the indentation a bit and see if the tests are green, we should be okay to merge it after that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great to hear. OK I'll have this ready in a sec...

@definitelynotsoftware
Copy link
Contributor Author

e2e tests are coming back green - are we good to go?

@caitp
Copy link
Contributor

caitp commented Aug 22, 2014

yeah works for me

caitp pushed a commit that referenced this pull request Aug 22, 2014
Helpful for people new to Angular to see the ng-app declaration in context with the expression
example. This will help illustrate the "Important thing to notice" point which follows: "The
reference to myApp module in <html ng-app="myApp">. This is what bootstraps the app using your
module."

Closes #8673
@caitp caitp closed this in b8b8411 Aug 22, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants