GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — docs: refactor routing doc by kapunahelewong · Pull Request #35566 · angular/angular

Conversation

@kapunahelewong

@kapunahelewong kapunahelewong commented Feb 19, 2020

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
  • 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
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@googlebot googlebot added the cla: yes label Feb 19, 2020
@pullapprove
pullapprove Bot requested a review from atscott February 19, 2020 22:45
@kapunahelewong kapunahelewong added comp: docs effort3: weeks risk: medium type: bug/fix labels Feb 19, 2020
@ngbot ngbot Bot modified the milestone: Backlog Feb 19, 2020
@kapunahelewong kapunahelewong changed the title docs: refactor routing doc [WIP]docs: refactor routing doc Feb 19, 2020
@kapunahelewong kapunahelewong added the aio: preview label Feb 19, 2020
@mary-poppins

mary-poppins commented Feb 19, 2020

Copy link
Copy Markdown

You can preview 361cbd0 at https://pr35566-361cbd0.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Feb 20, 2020

Copy link
Copy Markdown

You can preview a545f6b at https://pr35566-a545f6b.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Feb 27, 2020

Copy link
Copy Markdown

You can preview 8e1340d at https://pr35566-8e1340d.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Feb 28, 2020

Copy link
Copy Markdown

You can preview c844c1a at https://pr35566-c844c1a.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Mar 5, 2020

Copy link
Copy Markdown

You can preview cc3f85f at https://pr35566-cc3f85f.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kapunahelewong kapunahelewong changed the title [WIP]docs: refactor routing doc docs: refactor routing doc Mar 5, 2020
@pullapprove
pullapprove Bot requested a review from kara March 5, 2020 18:53
@kapunahelewong
kapunahelewong requested a review from aikithoughts March 10, 2020 18:42
@aikithoughts aikithoughts mentioned this pull request Mar 10, 2020
Rework nav to improve startup experience #35533
Closed
4 of 14 tasks
@mary-poppins

mary-poppins commented Mar 10, 2020

Copy link
Copy Markdown

You can preview bd4fd20 at https://pr35566-bd4fd20.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aikithoughts aikithoughts added the state: needs doc team review label Mar 27, 2020
atscott
atscott requested changes Mar 27, 2020
View reviewed changes
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
atscott
atscott reviewed Mar 31, 2020
View reviewed changes

@atscott atscott left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Approved from technical standpoint. I can re-review if there are changes requested from Dave.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread aio/content/examples/router/src/app/auth/auth.guard.2.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/auth/auth.guard.3.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/auth/auth.guard.4.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/auth/auth.guard.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 1, 2020

Copy link
Copy Markdown

You can preview a47d9c1 at https://pr35566-a47d9c1.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aikithoughts
aikithoughts reviewed Apr 2, 2020
View reviewed changes

@aikithoughts aikithoughts left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am about halfway through my review, but thought I'd send you these comments to get started.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kapunahelewong reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 6, 2020

Copy link
Copy Markdown

You can preview 4e89666 at https://pr35566-4e89666.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aikithoughts
aikithoughts approved these changes Apr 6, 2020
View reviewed changes

@aikithoughts aikithoughts left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

One last change. I think this is enough for this PR. Next steps, however, include:

  1. Refining the section on accessing query parameters to be as straightforward/minimal as possible.
  2. Move the content on lazy loading and preloading into the Lazy Loading topic.
  3. Move the tutorial to a new top-level section, Tutorials, in the guide.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
atscott
atscott approved these changes Apr 6, 2020
View reviewed changes

@atscott atscott left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Reviewed-for: fw-router

LGTM with a few additional nits

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
gkalpak
gkalpak suggested changes Apr 9, 2020
View reviewed changes
Comment thread aio/content/guide/router.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 9, 2020

Copy link
Copy Markdown

You can preview 897f521 at https://pr35566-897f521.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 13, 2020

Copy link
Copy Markdown

You can preview faeb1e5 at https://pr35566-faeb1e5.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 13, 2020

Copy link
Copy Markdown

You can preview 4639966 at https://pr35566-4639966.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@kapunahelewong kapunahelewong added target: patch This PR is targeted for the next patch release and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Apr 13, 2020
gkalpak
gkalpak approved these changes Apr 13, 2020
View reviewed changes

@gkalpak gkalpak left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thx for reducing the hard-coded code snippets, @kapunahelewong.
I've left some minor comments (and a couple of suggestions for future PRs) - otherwise lgtm ✨

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread packages/router/src/events.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/auth/auth.guard.3.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/auth/auth.guard.2.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app.component.8.html Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app.component.4.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app.component.4.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app.component.4.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app.component.1.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/examples/router/src/app/app-routing.module.7.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread aio/content/guide/router.md Outdated

@gkalpak gkalpak Apr 13, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This could be extracted into a separate docs (a router glossary).
Not necessary on this PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@kapunahelewong kapunahelewong Apr 14, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Agreed and it will be moved into the glossary in a follow up PR. Thanks!!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@rockument69 rockument69 Apr 23, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@kapunahelewong : I don't think these are glossary-style terms. They are in code font and should probably link to APIs, such as RouterModule. I could see possibly updating the existing Glossary entry to include more terminology.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gkalpak gkalpak added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 13, 2020
@kapunahelewong kapunahelewong added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews state: needs doc team review labels Apr 13, 2020
@mary-poppins

mary-poppins commented Apr 14, 2020

Copy link
Copy Markdown

You can preview 1a04e38 at https://pr35566-1a04e38.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mary-poppins

mary-poppins commented Apr 14, 2020

Copy link
Copy Markdown

You can preview 484e9f9 at https://pr35566-484e9f9.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@petebacondarwin
petebacondarwin removed their request for review April 14, 2020 20:41
@kapunahelewong
kapunahelewong removed the request for review from clydin April 14, 2020 21:58
@pullapprove
pullapprove Bot requested a review from clydin April 14, 2020 21:58
clydin
clydin approved these changes Apr 14, 2020
View reviewed changes
@kara
kara removed their request for review April 14, 2020 22:19
@pullapprove
pullapprove Bot requested a review from kara April 14, 2020 22:19
@kapunahelewong kapunahelewong removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Apr 15, 2020
@mary-poppins

mary-poppins commented Apr 15, 2020

Copy link
Copy Markdown

You can preview ec661b5 at https://pr35566-ec661b5.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kara
kara approved these changes Apr 15, 2020
View reviewed changes

@kara kara left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Still LGTM for the browser-support page

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kapunahelewong reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@kapunahelewong kapunahelewong added the action: merge The PR is ready for merge by the caretaker label Apr 15, 2020
@kapunahelewong
docs: refactor routing doc
745d624 
This rewrite changes headings to focus on user tasks rather than features,
verifies that content is up-to-date and complete, removes colloquial phrases,
adds prerequisites, and expands on a task-based section in the beginning
(a quick reference).
@mary-poppins

mary-poppins commented Apr 16, 2020

Copy link
Copy Markdown

You can preview 745d624 at https://pr35566-745d624.ngbuilds.io/.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@atscott atscott closed this in 305b059 Apr 16, 2020
atscott pushed a commit that referenced this pull request Apr 16, 2020
@kapunahelewong @atscott
docs: refactor routing doc (#35566)
1e4fb74 
This rewrite changes headings to focus on user tasks rather than features,
verifies that content is up-to-date and complete, removes colloquial phrases,
adds prerequisites, and expands on a task-based section in the beginning
(a quick reference).

PR Close #35566
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented May 24, 2020

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators May 24, 2020
@kapunahelewong
kapunahelewong deleted the router-refactor branch May 26, 2020 15:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@atscott atscott atscott approved these changes
@clydin clydin clydin approved these changes
+4 more reviewers
@gkalpak gkalpak gkalpak approved these changes
@rockument69 rockument69 rockument69 left review comments
@kara kara kara approved these changes
@aikithoughts aikithoughts aikithoughts approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@kapunahelewong kapunahelewong

Labels

action: merge The PR is ready for merge by the caretaker cla: yes effort3: weeks risk: medium target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@kapunahelewong @mary-poppins @atscott @kara @gkalpak @clydin @aikithoughts @rockument69 @googlebot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.