Subscriptions: dialog re-renderer #14076
Conversation
| @@ -51,29 +51,40 @@ export class LocalSubscriptionPlatformRenderer { | |||
| } | |||
|
|
|||
| /** | |||
| * | |||
| * @param {!Node} rootNode | |||
There was a problem hiding this comment.
Not used as far as I can tell?
|
|
||
| beforeEach(() => { | ||
| win = env.win; | ||
| doc = win.document; | ||
| ampdoc = env.ampdoc; | ||
| templatesMock = sandbox.mock(Services.templatesFor(win)); | ||
| actionRendererStub = sandbox.stub(LocalRenderer, 'renderActions').callsFake( |
There was a problem hiding this comment.
Not sure if this will actually work as a testing in all cases. The module stubbing is not possible. I think you need to re-factor it as a separate API on the instance of Renderer class and pass that instance to the dialog renderer. Then you can easily stub it here.
There was a problem hiding this comment.
Combined both files and worked
| @@ -110,6 +110,9 @@ const WHITELISTED_ATTRS = [ | |||
| /* Attributes added for amp-bind */ | |||
| // TODO(kmh287): Add more whitelisted attributes for bind? | |||
| 'text', | |||
| /* Attributes for amp-subscriptions */ | |||
| 'subscriptions-action', | |||
There was a problem hiding this comment.
Also need subscriptions-actions and subscriptions-display, right?
Can you please also add a test for this?
There was a problem hiding this comment.
Yep, Done and added the test
| .to.equal('<div subscriptions-section="actions">link</div>'); | ||
| expect(sanitizeHtml('<div subscriptions-actions="">link</div>')) | ||
| .to.equal('<div subscriptions-actions="">link</div>'); | ||
| expect(sanitizeHtml('<div subscriptions-display="">link</div>')) |
There was a problem hiding this comment.
Please add one more with some value for subscriptions-dialog attribute.
templatebefore dialog is shown