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(Getting Started): Updated material Getting Started guide to use install schematics and simplify the overall process #16654

Merged
merged 4 commits into from
Aug 22, 2019

Conversation

manughub
Copy link
Contributor

Closes #16426 (Rewrite getting started guide)

@manughub manughub requested a review from jelbourn as a code owner July 31, 2019 18:58
@googlebot

This comment has been minimized.

@googlebot googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jul 31, 2019
@manughub manughub force-pushed the master branch 2 times, most recently from 86041ab to 0963296 Compare July 31, 2019 20:32
@googlebot

This comment has been minimized.

@googlebot googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jul 31, 2019
@jelbourn jelbourn added docs This issue is related to documentation pr: merge safe target: patch This PR is targeted for the next patch release labels Aug 1, 2019
@jelbourn jelbourn requested a review from Splaktar August 1, 2019 01:01
@Splaktar
Copy link
Member

Splaktar commented Aug 1, 2019

The failing ci/circleci: tests_browserstack check appears to be due to the flaky Edge tests that Kristiyan is trying to fix in PR #16625.

It looks like I don't have permission to re-run the workflow, but that should hopefully resolve itself after your next push.

@Splaktar
Copy link
Member

Splaktar commented Aug 2, 2019

@manughub
Copy link
Contributor Author

manughub commented Aug 5, 2019

All review comments addressed.

Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

The initial/primary commit message should include the following footer:

Fixes #14380. Fixes #16426.

```

Alternatively, you can create a separate NgModule that imports and then re-exports all of the Angular Material components that you will use in your application. By exporting them again, other modules can simply include your `CustomMaterialModule` wherever Material components are needed, and automatically get all of the exported Material modules. A good place for importing/exporting the application-wide Material modules is the [SharedModule](https://angular.io/guide/ngmodule-faq#sharedmodule).
You need to import the `MatSliderModule` that you want to display by adding the following lines to your app.module.ts file.
Copy link
Member

Choose a reason for hiding this comment

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

I think that it may be a better getting started experience for new users if they generated a nav via the nav schematic (ng g @angular/material:nav nav).

They would only need one additional step to replace their app.component.html with the <app-nav></app-nav> component after they created it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discussed this with Jeremy and we decided to go with the MatSlider as the nav looks a bit out of place on the demo page. I'm back in the office and will address the other comments and submit my changes.

@Splaktar Splaktar added the P2 The issue is important to a large percentage of users, with a workaround label Aug 10, 2019
@jelbourn
Copy link
Member

FYI Manu's OOO this week

@manughub
Copy link
Contributor Author

Review comments addressed.

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Aug 22, 2019
@jelbourn jelbourn dismissed Splaktar’s stale review August 22, 2019 23:04

Comments addressed

@jelbourn jelbourn merged commit ca7fff5 into angular:master Aug 22, 2019
andrewseguin pushed a commit that referenced this pull request Aug 26, 2019
…install schematics and simplify the overall process (#16654)

* docs: rewrite getting started guide to use CLI schematics

* docs(Getting Started): Moving HammerJS install to appendix

* docs(Getting Started): Moving HammerJS install to appendix

* docs(Getting Started): Moving HammerJS install to appendix

(cherry picked from commit ca7fff5)
mmalerba pushed a commit to mmalerba/components that referenced this pull request Aug 27, 2019
…install schematics and simplify the overall process (angular#16654)

* docs: rewrite getting started guide to use CLI schematics

* docs(Getting Started): Moving HammerJS install to appendix

* docs(Getting Started): Moving HammerJS install to appendix

* docs(Getting Started): Moving HammerJS install to appendix
Splaktar added a commit that referenced this pull request Sep 16, 2019
improvements to header hierarchy and auto-generated anchors
- new anchor `/guide/getting-started#step-1`
- old anchor `/guide/getting-started#step-1-install-angular-material-npm-packages`
add link to hammerjs appendix entry
builds upon changes in PR #16654
Splaktar added a commit that referenced this pull request Sep 16, 2019
improvements to header hierarchy and auto-generated anchors
- new anchor `/guide/getting-started#step-1`
- old anchor `/guide/getting-started#step-1-install-angular-material-npm-packages`
add link to hammerjs appendix entry
builds upon changes in PR #16654
Splaktar added a commit that referenced this pull request Sep 16, 2019
improvements to header hierarchy and auto-generated anchors
- new anchor `/guide/getting-started#step-1`
- old anchor `/guide/getting-started#step-1-install-angular-material-npm-packages`
add link to hammerjs appendix entry
builds upon changes in PR #16654
jelbourn pushed a commit that referenced this pull request Sep 16, 2019
improvements to header hierarchy and auto-generated anchors
- new anchor `/guide/getting-started#step-1`
- old anchor `/guide/getting-started#step-1-install-angular-material-npm-packages`
add link to hammerjs appendix entry
builds upon changes in PR #16654
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement docs This issue is related to documentation P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite Getting Started Guide
4 participants