Skip to content
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

docs: add recipe with angular elements #1819

Closed
wants to merge 0 commits into from
Closed

docs: add recipe with angular elements #1819

wants to merge 0 commits into from

Conversation

santoshyadavdev
Copy link
Contributor

@santoshyadavdev santoshyadavdev commented May 4, 2019

Fixes #1621

PR Checklist

Please check if your PR fulfills the following requirements:

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
[ ] Other... Please describe:

What is the current behavior?

Closes #1621

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[X] No

Other information

@santoshyadavdev
Copy link
Contributor Author

added example code, docs changes in progress.

@santoshyadavdev
Copy link
Contributor Author

santoshyadavdev commented May 5, 2019

@wesleygrimes this is open for review, build issue will be resolved once #1795 is merged,

Copy link
Contributor

@wesleygrimes wesleygrimes left a comment

Choose a reason for hiding this comment

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

Here is my first pass review. Some work to do, but a great start.

@santoshyadavdev
Copy link
Contributor Author

Thanks a lot, @wesleygrimes ,
Will work on the changes. Again a lot of grammar mistake. Will Improve

@wesleygrimes
Copy link
Contributor

wesleygrimes commented May 5, 2019 via email

@santoshyadavdev
Copy link
Contributor Author

Great work! Have you tried using Grammarly or Antidote?

-Wes
On May 5, 2019, at 4:10 PM, Santosh Yadav @.***> wrote: Thanks a lot, @wesleygrimes , Will work on the changes. Again a lot of grammar mistake. Will Improve — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Thanks have started using it.

@wesleygrimes
Copy link
Contributor

@santoshyadav198613 Your latest updates were done using the amend or force push method. When these methods are used, we lose the ability to easily track changes in our PR review process. Next time around, can you just push additional new commits?

@santoshyadavdev
Copy link
Contributor Author

@santoshyadav198613 Your latest updates were done using the amend or force push method. When these methods are used, we lose the ability to easily track changes in our PR review process. Next time around, can you just push additional new commits?

Sure.

@wesleygrimes
Copy link
Contributor

@santoshyadav198613 the latest build failed due to incorrect path to example file:

Error processing docs: Error: Missing example file... relativePath: "store-elements/projects/my-counter/src/lib/app/counter.module.ts". - doc "guide/store/recipes/custom-elements" (content) - from file "guide/store/recipes/custom-elements.md"

There is no store-elements/projects/my-counter/src/lib/app/ folder so consider changing to

-<code-example header="src/lib/counter.module.ts" path="store-elements/projects/my-counter/src/lib/app/counter.module.ts">
+<code-example header="src/lib/counter.module.ts" path="store-elements/projects/my-counter/src/lib/counter.module.ts">

@santoshyadavdev
Copy link
Contributor Author

@santoshyadav198613 the latest build failed due to incorrect path to example file:

Error processing docs: Error: Missing example file... relativePath: "store-elements/projects/my-counter/src/lib/app/counter.module.ts". - doc "guide/store/recipes/custom-elements" (content) - from file "guide/store/recipes/custom-elements.md"

There is no store-elements/projects/my-counter/src/lib/app/ folder so consider changing to

-<code-example header="src/lib/counter.module.ts" path="store-elements/projects/my-counter/src/lib/app/counter.module.ts">
+<code-example header="src/lib/counter.module.ts" path="store-elements/projects/my-counter/src/lib/counter.module.ts">

Hi @wesleygrimes,

Don't review as of now, I only pushed these changes as I don't want to keep it locally. Will let you know once done.

@wesleygrimes wesleygrimes added WIP Not ready for review Comp: Docs labels May 8, 2019
@ngrxbot
Copy link
Collaborator

ngrxbot commented May 8, 2019

Preview docs changes for d3720f8 at https://previews.ngrx.io/pr1819-d3720f8/

@santoshyadavdev
Copy link
Contributor Author

Hi @wesleygrimes ,
Open for review now, wanted to ask one thing, I have not added anything about counter increment, decrement and reset component.

Also as the code cannot run directly on stackblitz, I have added a download link.

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 8, 2019

Preview docs changes for e2b11e3 at https://previews.ngrx.io/pr1819-e2b11e3/

@wesleygrimes
Copy link
Contributor

Hi @wesleygrimes ,
Open for review now, wanted to ask one thing, I have not added anything about counter increment, decrement and reset component.

Also as the code cannot run directly on stackblitz, I have added a download link.

Thanks @santoshyadav198613, we will review tomorrow.

Copy link
Contributor

@wesleygrimes wesleygrimes left a comment

Choose a reason for hiding this comment

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

Final round of changes

@wesleygrimes wesleygrimes self-requested a review May 10, 2019 14:59
Copy link
Contributor

@wesleygrimes wesleygrimes left a comment

Choose a reason for hiding this comment

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

Couple more changes

Copy link
Contributor

@wesleygrimes wesleygrimes left a comment

Choose a reason for hiding this comment

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

Is this ready for a review @santoshyadav198613?

We just want to be sure because it seems like there are some commits appended to this PR that are already merged (Example app and @ngrx/data).

It also seems that the .spec files crept back in.

@santoshyadavdev
Copy link
Contributor Author

HI @wesleygrimes ,

I had to force push as after rebase from master there were extra files which were coming in my commit.

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 11, 2019

Preview docs changes for b3f6083 at https://previews.ngrx.io/pr1819-b3f6083/

@santoshyadavdev
Copy link
Contributor Author

Hi @wesleygrimes ,
You can remove WIP tag now,

@wesleygrimes
Copy link
Contributor

wesleygrimes commented May 12, 2019 via email

@santoshyadavdev
Copy link
Contributor Author

Is it ready for review?

On May 12, 2019, at 3:00 PM, Santosh Yadav @.***> wrote: Hi @wesleygrimes , You can remove WIP tag now, — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Yes, it is.

@santoshyadavdev
Copy link
Contributor Author

@santoshyadav198613 just a couple more tweaks. Feedback left on comments.

Done with the changes.

@wesleygrimes
Copy link
Contributor

wesleygrimes commented May 15, 2019 via email

@wesleygrimes
Copy link
Contributor

wesleygrimes commented May 15, 2019 via email

@santoshyadavdev
Copy link
Contributor Author

santoshyadavdev commented May 15, 2019

Not for download, just point them to the Repo for viewing, they can decide to download later from GitHub if needed.

On Wed, May 15, 2019 at 1:21 PM Santosh Yadav @.> wrote: @.* commented on this pull request. ------------------------------ In projects/ngrx.io/content/guide/store/recipes/custom-elements.md <#1819 (comment)>: > @@ -0,0 +1,88 @@ +# Using Store with Angular Elements +## Creating Angular Elements +The following recipe shows you how to manage the state of a counter, and how to select and display it within an Angular Elements and use the same within an Angular Project and a static page. +## Recipe +You can download Angular Elements Project. But won't that be an overhead for users as they have to download the entire repo? By download link, we are downloading the same app, and they will get the only code they want. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1819?email_source=notifications&email_token=AACPFVERQAAWFFFYWXUIDEDPVRBARA5CNFSM4HKZSIR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBYXQGFI#discussion_r284364675>, or mute the thread https://github.com/notifications/unsubscribe-auth/AACPFVBZX35XD7HNZ55SMRLPVRBARANCNFSM4HKZSIRQ .
-- Thanks, Wes

Is below text fine?
You can refer the code from GitHub Repo and download it.

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 15, 2019

Preview docs changes for ba80559 at https://previews.ngrx.io/pr1819-ba80559/

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 15, 2019

Preview docs changes for d5fbaba at https://previews.ngrx.io/pr1819-d5fbaba/

1 similar comment
@ngrxbot
Copy link
Collaborator

ngrxbot commented May 15, 2019

Preview docs changes for d5fbaba at https://previews.ngrx.io/pr1819-d5fbaba/

@brandonroberts brandonroberts added the Needs Cleanup Review changes needed label May 15, 2019
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Mostly formatting requests.
The config files aren't added in the other examples, do you need them for a specific reason?

@@ -0,0 +1,96 @@
# Using Store with Angular Elements
## Creating Angular Elements
The following recipe illustrates utilizing Angular Elements with a NgRx Store to manage the state of a counter, and select and display that counter state from within separate Angular Elements. This is all accomplished from within a single Angular Project and static HTML page.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The following recipe illustrates utilizing Angular Elements with a NgRx Store to manage the state of a counter, and select and display that counter state from within separate Angular Elements. This is all accomplished from within a single Angular Project and static HTML page.
The following recipe illustrates utilizing Angular Elements with a NgRx Store. This provides the ability to manage the state of a counter, and to select and display that counter state from within separate Angular Elements. This is all accomplished from within a single Angular Project and static HTML page.

@santoshyadavdev
Copy link
Contributor Author

Hi @timdeschryver , changes done.

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 16, 2019

Preview docs changes for efe1ff9 at https://previews.ngrx.io/pr1819-efe1ff9/

@timdeschryver
Copy link
Member

Sorry but I think not all of is done @santoshyadav198613 ? For example, the empty ngOnInit functions and the line breaks around the code blocks.

@santoshyadavdev
Copy link
Contributor Author

Ohh sorry, will fix it.

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 19, 2019

Preview docs changes for 9f52f81 at https://previews.ngrx.io/pr1819-9f52f81/

@santoshyadavdev
Copy link
Contributor Author

Sorry but I think not all of is done @santoshyadav198613 ? For example, the empty ngOnInit functions and the line breaks around the code blocks.

Done actually, I only did changes related to md file in the last check-in. You can verify now.

@santoshyadavdev
Copy link
Contributor Author

Done @wesleygrimes .

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 20, 2019

Preview docs changes for 8e6649f at https://previews.ngrx.io/pr1819-8e6649f/

@wesleygrimes
Copy link
Contributor

Done @wesleygrimes .

OK, did you review all of @timdeschryver comments? I marked resolved the ones that appeared to be resolved, but I wasn't sure about the line breaks for code blocks. Are those resolved?

@santoshyadavdev
Copy link
Contributor Author

Done @wesleygrimes .

OK, did you review all of @timdeschryver comments? I marked resolved the ones that appeared to be resolved, but I wasn't sure about the line breaks for code blocks. Are those resolved?

Yes resolved them as well.

@santoshyadavdev
Copy link
Contributor Author

Hi @wesleygrimes @timdeschryver let me know if anything needed here.

@wesleygrimes
Copy link
Contributor

Hi @wesleygrimes @timdeschryver let me know if anything needed here.

We will get back to you and let you know. Thanks.

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Should we update the example with the creator functions?

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Do we need the empty projects/ngrx.io/content/examples/store-elements/example-config.json file?

@@ -0,0 +1,2 @@

Copy link
Member

Choose a reason for hiding this comment

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

Remove empty line

Suggested change

})
export class AppModule {

ngDoBootstrap() {
Copy link
Member

@timdeschryver timdeschryver Jun 13, 2019

Choose a reason for hiding this comment

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

But there is no logic in this function?

Edit: GitHub seems to not like this, I'm asking this question because of this conversation before:

I asked if this this needed, where you replied:

Yes to bootstrap it manually so our angular elements can work.


```json
"scripts": [
{"input": "node_modules/document-register-element/build/document-register-element.js"}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{"input": "node_modules/document-register-element/build/document-register-element.js"}
{ "input": "node_modules/document-register-element/build/document-register-element.js "}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HI @timdeschryver ,
Yes ngBootStrap is needed to build element. if we remove the elements will not work.

@santoshyadavdev
Copy link
Contributor Author

Should we update the example with the creator functions?
Yes let me do that.

@ngrxbot
Copy link
Collaborator

ngrxbot commented Jun 23, 2019

Preview docs changes for a7a565c at https://previews.ngrx.io/pr1819-a7a565c/

@ngrxbot
Copy link
Collaborator

ngrxbot commented Jun 23, 2019

Preview docs changes for 38bfefb at https://previews.ngrx.io/pr1819-38bfefb/

@santoshyadavdev
Copy link
Contributor Author

Hi @timblakely , @wesleygrimes due to some issue with commit this got closed opened a new one https://github.com/ngrx/platform/pull/1967/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Comp: Docs Needs Cleanup Review changes needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs: Add recipe guide for using Store with custom elements
5 participants