Skip to content

refactor(angular_1_router): Added value for router root component#5052

Closed
brandonroberts wants to merge 1 commit intoangular:masterfrom
brandonroberts:router1-root-component
Closed

refactor(angular_1_router): Added value for router root component#5052
brandonroberts wants to merge 1 commit intoangular:masterfrom
brandonroberts:router1-root-component

Conversation

@brandonroberts
Copy link
Copy Markdown
Contributor

@brandonroberts brandonroberts commented Nov 1, 2015

Allows for overriding of the root component that is used to configure routes.

Closes #4965

@btford
Copy link
Copy Markdown
Contributor

btford commented Nov 2, 2015

LGTM, but this needs a test.

@btford btford self-assigned this Nov 2, 2015
@naomiblack naomiblack modified the milestone: beta-00 Nov 2, 2015
@brandonroberts
Copy link
Copy Markdown
Contributor Author

brandonroberts commented Nov 2, 2015

@btford I updated the PR with a test for the root component.

btford
btford reviewed Nov 26, 2015
View reviewed changes
modules/angular1_router/test/integration/navigation_spec.js Outdated
Copy link
Copy Markdown
Contributor

@btford btford Nov 26, 2015

Choose a reason for hiding this comment

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

This changes the behavior of all of the tests, which is undesirable. It'd be better to move these tests into a distinct describe block to isolate the behavior.

@btford
Copy link
Copy Markdown
Contributor

btford commented Nov 26, 2015

This is a good change, but it probably needs to be rebased due to my negligence (sorry about that :( ).

If you rebase and address the one issue I pointed out, I'll make sure this gets merged.

Thanks!

@brandonroberts
Copy link
Copy Markdown
Contributor Author

brandonroberts commented Nov 26, 2015

@btford I rebased and moved the the test into a separate describe. Thanks for your help!

@brandonroberts brandonroberts force-pushed the router1-root-component branch 3 times, most recently from 62df101 to 284e1d0 Compare December 2, 2015 20:13
@brandonroberts
Copy link
Copy Markdown
Contributor Author

brandonroberts commented Dec 3, 2015

@btford I rebased again as this one got stale.

@btford btford added pr_state: LGTM action: merge The PR is ready for merge by the caretaker and removed state: Follow Up labels Dec 3, 2015
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Dec 3, 2015

Merging PR #5052 on behalf of @jelbourn to branch presubmit-jelbourn-pr-5052.

@btford
Copy link
Copy Markdown
Contributor

btford commented Dec 3, 2015

Thanks so much @brandonroberts! This is on its way to master. 💃

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Dec 3, 2015

Merging PR #5052 on behalf of @jelbourn to branch presubmit-jelbourn-pr-5052.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Dec 3, 2015

Merging PR #5052 on behalf of @jelbourn to branch presubmit-jelbourn-pr-5052.

@mhevery mhevery closed this in 478a25e Dec 3, 2015
@brandonroberts brandonroberts deleted the router1-root-component branch December 12, 2015 16:19
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 7, 2019

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 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@btford btford

Labels

action: merge The PR is ready for merge by the caretaker cla: yes

Projects

None yet

Milestone

beta.0

Development

Successfully merging this pull request may close these issues.

feat(router 1.x): Add ability to define root component value

6 participants

@brandonroberts @btford @mary-poppins @jelbourn @naomiblack @googlebot