glade ad element with a single gladeReady listener #2
Conversation
|
LGTM |
|
+bicknellr Hi Russell, in this PR there are two versions of the element: |
glade-ad-single-listener.html
Outdated
| } | ||
| </style> | ||
| <div class="ad_slot_placeholder"></div> | ||
| </div> |
glade-ad-single-listener.html
Outdated
| </template> | ||
| <script> | ||
| (function() { | ||
| var ad_slot; |
There was a problem hiding this comment.
Left over from playing :) Fixed.
glade-ad-single-listener.html
Outdated
| properties: { | ||
| adSlotId: { | ||
| type: String, | ||
| }, |
There was a problem hiding this comment.
Properties that only specify type have a shorthand form you can use if you want:
adSlotId: {type: String}, -> adSlotId: String,
glade-ad-single-listener.html
Outdated
| }, | ||
|
|
||
| observers: [ | ||
| 'propertiesChanged(adSlotId, dataAdUnitPath, width, height)' |
There was a problem hiding this comment.
propertiesChanged no longer exists, so I think you can remove this whole observers list.
(tangent) If I remember correctly, all of the attributes will have been set to their respective properties by the time ready is called. Given that a <glade-ad> always waits until ready before it attempts to run _load (either running sync or inserting itself into the queue to be called later), all the properties should be available in time.
There was a problem hiding this comment.
Got it; removing the observers list.
glade-ad-single-listener.html
Outdated
| _load: function() { | ||
| Polymer.RenderStatus.afterNextRender(this, function() { | ||
|
|
||
| var ad_slot = Polymer.dom(this.root).querySelector(".ad_slot_placeholder"); |
There was a problem hiding this comment.
If you change <div class="ad_slot_placeholder"></div> above to <div id="ad_slot_placeholder"></div> (since you only have one), you can access it here as this.$.ad_slot_placeholder. Polymer will find elements with ids in your template and add them as properties of this.$ automatically for you so you don't need to querySelector manually.
|
Thanks +bicknellr; we incorporated your recommendations. |
|
+demianrenzulli The component seems to be working nicely. Let's make some more tests to verify. Thanks! |
No description provided.