fix(router): Ensure all outlets are used when commands has a prefix#39456
Closed
atscott wants to merge 3 commits intoangular:masterfrom
Closed
fix(router): Ensure all outlets are used when commands has a prefix#39456atscott wants to merge 3 commits intoangular:masterfrom
atscott wants to merge 3 commits intoangular:masterfrom
Conversation
6fabfbf to
0b506b3
Compare
Contributor
Author
0b506b3 to
896aaf6
Compare
Contributor
|
My opinion ofc but I think it makes more sense to add the |
Contributor
Author
|
@alfaproject - That's a fair suggestion and I think different people will have different opinions on the matter. I actually like having a set of tests that are commented out/ignored but can be enabled when a fix becomes available. My feeling is that tests act as a form of documentation and the Either way, I'm not aware of any style guide on this, so I'll just leave it as-is. |
896aaf6 to
9dbd5c6
Compare
This commit has a small refactor of some methods in create_url_tree.ts and adds some test cases, including two that will fail at the moment but should pass. A follow-up commit will make use of the refactorings to fix the test with minimal changes.
When there is a primary outlet present in the outlets map and the object is also prefixed with some other commands, the current logic only uses the primary outlet and ignores the others. This change ensures that all outlets are respected at the segment level when prefixed with other commands.
1bfe4f0 to
5346092
Compare
Contributor
Author
josephperrott
pushed a commit
that referenced
this pull request
Oct 30, 2020
…39456) When there is a primary outlet present in the outlets map and the object is also prefixed with some other commands, the current logic only uses the primary outlet and ignores the others. This change ensures that all outlets are respected at the segment level when prefixed with other commands. PR Close #39456
josephperrott
pushed a commit
that referenced
this pull request
Oct 30, 2020
…39456) When there is a primary outlet present in the outlets map and the object is also prefixed with some other commands, the current logic only uses the primary outlet and ignores the others. This change ensures that all outlets are respected at the segment level when prefixed with other commands. PR Close #39456
josephperrott
pushed a commit
that referenced
this pull request
Oct 30, 2020
…39456) When there is a primary outlet present in the outlets map and the object is also prefixed with some other commands, the current logic only uses the primary outlet and ignores the others. This change ensures that all outlets are respected at the segment level when prefixed with other commands. PR Close #39456
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Note: Easiest to review is per-commit.
Commit 1: A refactor only. No functional changes. Also documents behavior with tests and coherent example config.
Commit 2: Small change to allow tests from commit 1 to pass.
When there is a primary outlet present in the outlets map and the object is also prefixed
with some other commands, the current logic only uses the primary outlet and ignores
the others. This change ensures that all outlets are respected at the
segment level when prefixed with other commands.
Fixes issue that prevented navigation in #39401.
Partially addressed #13523 - That issue, along with #15718, stem from a general difficulty to change outlets because one has to specify the change using the correct segment 'level' (see tests named
should not clear secondary outlet when at root and prefix is usedandshould not clear non-root secondary outlet when command is targeting root). Additionally, it's still not possible to change two outlets that appear at different levels in the config in a single navigation.