This repository was archived by the owner on Dec 8, 2022. It is now read-only.
Child route support, root guard support #177
Merged
Conversation
This file contains 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
Codecov Report
@@ Coverage Diff @@
## master #177 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 38 38
Lines 871 928 +57
Branches 116 130 +14
=====================================
+ Hits 871 928 +57
Continue to review full report at Codecov.
|
blackbaud-brandonstirnaman
force-pushed
the
child-routes
branch
from
a0d5ebd to
194255a
Compare
Blackbaud-BobbyEarl
approved these changes
Jun 13, 2017
e2e/shared/common.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Verify directory exists in src/app folder |
There was a problem hiding this comment.
Same comment as the verifyAppFolder method.
lib/sky-pages-route-generator.js
Outdated
| @@ -106,6 +106,14 @@ function generateRoutes(skyAppConfig) { | |||
| .sync(path.join(skyAppConfig.runtime.srcPath, skyAppConfig.runtime.routesPattern)) | |||
| .map(file => parseFileIntoEntity(skyAppConfig, file, counter++)); | |||
|
|
|||
| // Add a root component that will become the wrapper for all the others | |||
| entities.push({ | |||
| routePath: ['~'], // we need a non-empty route path so it won't be merged later | |||
There was a problem hiding this comment.
Not that I would expect it, but any concern if a user has a ~ folder?
‘?’ is an illegal character for windows paths and incredibly unlikely to be used.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
PR implements child routing based on the design @Blackbaud-PaulCrowder and I discussed.
A directory prefixed with # such as src/app/about/#child would create a child-route underneath /about which can be affected by canActivateChild guards as well as used by controls that implement router-outlet in their template such as the future tab control (or currently microedge contrib tab control).
A directory without the prefix but underneath a child route such as src/app/about/#child/top-level will break out and become a top level route that cannot be affected by canActivateChild guards and will not participate in router-outlet.
All other routes will continue to be top-level. Routes continue to only be created if an index.html is provided.
Additionally now that we have child routing, I've also implemented a top-level route that makes it very simple to declare an index.guard.ts next to src/app/index.html and have a guard that affects being able to load anything within the SPA. This should be the preferred method for locking down SPAs in future.
Added several e2e tests to validate the functionality as well as unit tests for the route generation.