Feature help library #270
Feature help library #270
Conversation
…on help, using help library instead of client, updated app component tests
Blackbaud-BrandonJones
changed the title
Codecov Report
@@ Coverage Diff @@
## master #270 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 57 57
Lines 1344 1353 +9
Branches 200 203 +3
=====================================
+ Hits 1344 1353 +9
Continue to review full report at Codecov.
|
|
Is the description of this PR still accurate @Blackbaud-BrandonJones? |
|
Nope sorry, will update. This PR is now entirely self sufficient. |
…randonJones/skyux-builder into feature-help-library
|
Updated and ready for your review @Blackbaud-BobbyEarl |
runtime/config.ts
Outdated
| @@ -47,7 +66,7 @@ export interface SkyuxConfig { | |||
| cssPath?: string; | |||
| command?: string; | |||
| compileMode?: string; | |||
| help?: any; | |||
| help?: HelpWidgetConfig; | |||
There was a problem hiding this comment.
I'm curious about the creation of this interface. We'd previous used interfaces for the auth and help clients. We decided against that; however, as somewhat painfully tied those libraries to builder, but without any benefit (consumers never use the interface).
There was a problem hiding this comment.
That makes sense. I added it because I was using it for the Help Library and thought builder may want to also make use of it. That being said, having to update it in multiple places should one change would be a pain, I am fine with removing it.
src/app/app.component.ts
Outdated
| @@ -74,6 +74,7 @@ export class AppComponent implements OnInit { | |||
| private windowRef: SkyAppWindowRef, | |||
| private config: SkyAppConfig, | |||
| private styleLoader: SkyAppStyleLoader, | |||
| private helpInitService: HelpInitializationService, | |||
There was a problem hiding this comment.
Could we use a similar approach to the optional searchProvider below this line, and mark the service as optional + check for it's existence for using it? I believe this would allow us to conditionally inject it in lib/sky-pages-module-generator.js, thereby not including the library for SPAs that don't have help defined in skyuxconfig.json. Thoughts?
There was a problem hiding this comment.
Good thoughts! I'll see what I can do.
lib/sky-pages-module-generator.js
Outdated
| @@ -83,6 +103,12 @@ function getSource(skyAppConfig) { | |||
| 'AppExtrasModule' | |||
| ]; | |||
|
|
|||
| if (skyAppConfig.skyux.help) { | |||
| nodeModulesImports.push(`import { BBHelpModule } from '@blackbaud/skyux-lib-help';`); | |||
There was a problem hiding this comment.
I'm nitpicking, but can you name this nodeModuleImports so the case matches runtimeModuleImports?
Working demo with setup instructions can be found here:
https://github.com/Blackbaud-BrandonJones/bb-help-test-skyux2