amp-apester: Support keyword lookup in AmpDocShadow#27345
amp-apester: Support keyword lookup in AmpDocShadow#27345mdmower merged 5 commits intoampproject:masterfrom
Conversation
Query meta[name="keywords"] using method supported by AmpDocShadow as well as other AmpDoc subtypes.
caroqliu
left a comment
There was a problem hiding this comment.
A few comments, thanks for making this change. Is there an issue to attach to this PR, or was the change motivated by general code health?
| .split(',') | ||
| .filter((e) => e) | ||
| .map((e) => e.trim()); | ||
| .map((e) => e.trim()) |
There was a problem hiding this comment.
Are the filter and map swapped here to catch whitespace-only entries?
There was a problem hiding this comment.
Seeing this far after the fact, but something I've seen Justin do that is really nice-looking is .filter(Boolean) :D
There was a problem hiding this comment.
Oh yeah, slick, I'll totally adopt that in the future. Thanks!
| articleMetaTags.length ? articleMetaTags : extratctTitle(root) || [] | ||
| articleMetaTags.length | ||
| ? articleMetaTags | ||
| : extratctTitle(ampdoc.getRootNode()) || [] |
There was a problem hiding this comment.
nit: This method name appears to be a typo, mind fixing it here and elsewhere?
| document.head.textContent = ''; | ||
| }); | ||
| afterEach(() => { | ||
| Services.ampdoc(document).meta_ = null; |
There was a problem hiding this comment.
Instead of accessing the private property, let's reset using setMetaByName -- keywords is the only one that needs to be reset for the purposes of tests, right?
There was a problem hiding this comment.
I suppose we could do that, but this is the most direct means of resetting the cached meta tags in an AmpDoc, see:
Lines 739 to 740 in d8be2b6
There was a problem hiding this comment.
OK, feel free to leave as is. If you could include the same comment that would be helpful too.
* amp-apester: Support keyword lookup in AmpDocShadow Query meta[name="keywords"] using method supported by AmpDocShadow as well as other AmpDoc subtypes. * extratctTitle --> extractTitle * Add test note about clearing meta_
Query meta[name="keywords"] using method supported by
AmpDocShadow as well as other AmpDoc subtypes.