Use #cloneNode to clone FixedLayer body#6721
Conversation
src/service/fixed-layer.js
Outdated
| const bodyId = doc.body.id; | ||
| this.fixedLayer_ = doc.createElement('body'); | ||
| this.fixedLayer_.id = bodyId || '-amp-fixedlayer'; | ||
| this.fixedLayer_ = doc.body.cloneNode(); |
There was a problem hiding this comment.
Can you add explicit /*deep*/ false here, to make it self documenting?
src/service/fixed-layer.js
Outdated
| this.fixedLayer_ = doc.createElement('body'); | ||
| this.fixedLayer_.id = bodyId || '-amp-fixedlayer'; | ||
| this.fixedLayer_ = doc.body.cloneNode(); | ||
| this.fixedLayer_.id += ' -amp-fixedlayer'; |
There was a problem hiding this comment.
This breaks the old fix. We need the actual body.id if one was specified.
There was a problem hiding this comment.
I'm not overriding, I'm appending.
There was a problem hiding this comment.
Yeah, I can see that. But that breaks the previous fix. All #bodyid .x will be broken by this. It should be fixedLayer_.id = doc.body.id || '-amp-fixedlayer'. Or, tbh, we can simply kill -amp-fixedlayer and always use body.id.
There was a problem hiding this comment.
Oh man, I though id was like class. 😳
Fixed.
There was a problem hiding this comment.
Cool. Do the push and I'll approve.
256b2e4 to
82334c2
Compare
src/service/fixed-layer.js
Outdated
| this.fixedLayer_ = doc.createElement('body'); | ||
| this.fixedLayer_.id = bodyId || '-amp-fixedlayer'; | ||
| this.fixedLayer_ = doc.body.cloneNode(/* deep */ false); | ||
| this.fixedlayer.removeAttribute('style'); |
There was a problem hiding this comment.
we should also be making class = "" ?
There was a problem hiding this comment.
No class is explicitly copied. For the same reason that we copy ID, to preserve the selectors.
82334c2 to
6dd9f5f
Compare
Re: ampproject#6706 (comment) Also removes a [temporary fix](ampproject#4097) that is [no longer necessary](ampproject#6033).
6a85796 to
2080c1e
Compare
2080c1e to
93aac3f
Compare
* Use #cloneNode to clone FixedLayer body Re: ampproject#6706 (comment) Also removes a [temporary fix](ampproject#4097) that is [no longer necessary](ampproject#6033). * Fix test
* Use #cloneNode to clone FixedLayer body Re: ampproject#6706 (comment) Also removes a [temporary fix](ampproject#4097) that is [no longer necessary](ampproject#6033). * Fix test
* Use #cloneNode to clone FixedLayer body Re: ampproject#6706 (comment) Also removes a [temporary fix](ampproject#4097) that is [no longer necessary](ampproject#6033). * Fix test
Re: #6706 (comment)
Also removes a temporary fix that is no longer necessary.