Context in popovers

[isaacplmann]

I added the ability to pass context into popover templates.

[pkozlowski-opensource]

Could you please share motivation for this change? Also please note that we can't merge any PRs without associated tests so adding those would be mandatory.

But yeh, before speaking about tests I would like to hear more about a use-case...

[isaacplmann]

I'm writing a product tour library - ng2-tour

I need to be able to pass in a template for the popover content that can have the context values inserted when the popover is shown. You can see in the demo page that the same template is used for all the tour steps, but the content text is different.

The template looks like this:

    <template #tourStep let-step="step">
      <p class="tour-step-content">{{step?.content}}</p>
      <div class="tour-step-navigation">
          // buttons, etc, go here...
      </div>
    </template>

That let-step directive will never get a value supplied to it unless a context is associated with the popover when it is created. Angular has the ability built in, you just weren't surfacing the API. Hence, the PR.

[pkozlowski-opensource]

I see, make sense. For this PR to go forward it would need:

• tests;
• tooltips update (we are trying to make both tooltip and popovers aligned but can't share more code till we switch to Angular 2.3.x);
• documentation / demo update.

Are you willing to put more work into it to push it forward?

[isaacplmann]

Working on it now. Can you point me to the tooltip/documentation files that need updated? I figure you know the organization better than I do.

[pkozlowski-opensource]

Can you point me to the tooltip/documentation files that need updated? I figure you know the organization better than I do.

Documentation is in tooltip / popover files (jsdoc). This is where you would have to document new @Input()s. Demos reside in https://github.com/ng-bootstrap/ng-bootstrap/tree/master/demo/src/app/components/popover and https://github.com/ng-bootstrap/ng-bootstrap/tree/master/demo/src/app/components/tooltip

Thnx!

[isaacplmann]

I added tests and docs. There weren't any new @inputs, just an optional new parameter to the open() function. The popover and tooltip pages now document the new optional context parameter and there's a new demo section on each page.

[isaacplmann]

Any feedback on the tests/docs?

[pkozlowski-opensource]

Wrong import

[pkozlowski-opensource]

Wrong import

[pkozlowski-opensource]

@isaacplmann could you please:

• fix imports (I've left comments in the code)
• squash all the commits into one
• amend the commit message to follow our conventions?

[isaacplmann]

Changed the imports to import from '@ng-bootstrap/ng-bootstrap'. And squashed the commits into one.

[pkozlowski-opensource]

@isaacplmann I was trying to pull in this change but the demo site fails with the following errors (there are similar ones for popovers):

ERROR in ./demo/src/app/components/tooltip/demos/tplwithcontext/tooltip-tplwithcontext.ts
(1,9): error TS2305: Module '"/home/pk/work/gitrepos/gh/pkozlowski-opensource/core/src/index"' has no exported member 'NgbTooltip'.

ERROR in ./~/prismjs-loader?lang=typescript!./demo/src/app/components/tooltip/demos/tplwithcontext/tooltip-tplwithcontext.ts
(1,9): error TS2305: Module '"/home/pk/work/gitrepos/gh/pkozlowski-opensource/core/src/index"' has no exported member 'NgbTooltip'.

I guess that the root cause is #779 so let me prepare a PR that exports all the directive instances. Then you will be able to rebase on top of it and things should work. Unless you want to take a stab at #779?

[isaacplmann]

It looks like you're already fixing this in #1154 . If you merge that into master, I'll rebase this PR.

[pkozlowski-opensource]

@isaacplmann #1154 landed, please rebase.

[isaacplmann]

Rebased. And the demo now works.

[pkozlowski-opensource]

Merged, thnx!