Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃毊 Remove closestAncestorElementByTag #21000

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion build-system/conformance-config.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ requirement: {

requirement: {
type: BANNED_PROPERTY_CALL
error_message: 'Use closestAncestorElementBySelector, closestAncestorElementByTag etc in src/dom.js'
error_message: 'Use closestAncestorElementBySelector in src/dom.js'
value: 'Element.prototype.closest'
whitelist: 'src/dom.js'
}
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-ad/0.1/amp-ad-custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {Services} from '../../../src/services';
import {addParamToUrl} from '../../../src/url';
import {
childElementByTag,
closestAncestorElementByTag,
closestAncestorElementBySelector,
removeChildren,
} from '../../../src/dom';
import {hasOwn} from '../../../src/utils/object';
Expand Down Expand Up @@ -211,7 +211,7 @@ export class AmpAdCustom extends AMP.BaseElement {

// Get the parent body of this amp-ad element. It could be the body of
// the main document, or it could be an enclosing iframe.
const body = closestAncestorElementByTag(this.element, 'BODY');
const body = closestAncestorElementBySelector(this.element, 'BODY');
const elements = body.querySelectorAll('amp-ad[type=custom]');
for (let index = 0; index < elements.length; index++) {
const elem = elements[index];
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-audio/0.1/amp-audio.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
} from '../../../src/mediasession-helper';
import {Layout} from '../../../src/layout';
import {assertHttpsUrl} from '../../../src/url';
import {closestAncestorElementByTag} from '../../../src/dom';
import {closestAncestorElementBySelector} from '../../../src/dom';
import {dev, user} from '../../../src/log';
import {getMode} from '../../../src/mode';
import {listen} from '../../../src/event-helper';
Expand Down Expand Up @@ -209,7 +209,7 @@ export class AmpAudio extends AMP.BaseElement {
* @private
*/
isStoryDescendant_() {
return closestAncestorElementByTag(this.element, 'AMP-STORY');
return closestAncestorElementBySelector(this.element, 'AMP-STORY');
}

/** @private */
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-auto-ads/0.1/placement.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
import {Services} from '../../../src/services';
import {clamp} from '../../../src/utils/math';
import {
closestAncestorElementByTag,
closestAncestorElementBySelector,
createElementWithAttributes,
scopedQuerySelectorAll,
whenUpgradedToCustomElement,
Expand Down Expand Up @@ -425,7 +425,7 @@ function isPositionValid(anchorElement, position) {
}
const elementToCheck = dev().assertElement(elementToCheckOrNull);
return !BLACKLISTED_ANCESTOR_TAGS.some(tagName => {
if (closestAncestorElementByTag(elementToCheck, tagName)) {
if (closestAncestorElementBySelector(elementToCheck, tagName)) {
user().warn(TAG, 'Placement inside blacklisted ancestor: ' + tagName);
return true;
}
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-bind/0.1/bind-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {ChunkPriority, chunk} from '../../../src/chunk';
import {RAW_OBJECT_ARGS_KEY} from '../../../src/action-constants';
import {Services} from '../../../src/services';
import {Signals} from '../../../src/utils/signals';
import {closestAncestorElementByTag, iterateCursor} from '../../../src/dom';
import {closestAncestorElementBySelector, iterateCursor} from '../../../src/dom';
import {debounce} from '../../../src/utils/rate-limit';
import {deepEquals, getValueForExpr, parseJson} from '../../../src/json';
import {deepMerge, dict, map} from '../../../src/utils/object';
Expand Down Expand Up @@ -1198,7 +1198,7 @@ export class Bind {
if (!Services.platformFor(this.win_).isSafari()) {
return;
}
const select = closestAncestorElementByTag(element, 'select');
const select = closestAncestorElementBySelector(element, 'select');
if (!select) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-date-picker/0.1/amp-date-picker.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {Layout, isLayoutSizeDefined} from '../../../src/layout';
import {Services} from '../../../src/services';
import {batchFetchJsonFor} from '../../../src/batched-json';
import {
closestAncestorElementByTag,
closestAncestorElementBySelector,
escapeCssSelectorIdent,
isRTL,
iterateCursor,
Expand Down Expand Up @@ -832,7 +832,7 @@ export class AmpDatePicker extends AMP.BaseElement {
return existingField;
}

const form = closestAncestorElementByTag(this.element, 'form');
const form = closestAncestorElementBySelector(this.element, 'form');
if (this.mode_ == DatePickerMode.STATIC && form) {
const hiddenInput = this.document_.createElement('input');
hiddenInput.type = 'hidden';
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-image-lightbox/0.1/amp-image-lightbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ class AmpImageLightbox extends AMP.BaseElement {
let caption = null;

// 1. Check <figure> and <figcaption>.
const figure = dom.closestAncestorElementByTag(sourceElement, 'figure');
const figure = dom.closestAncestorElementBySelector(sourceElement, 'figure');
if (figure) {
caption = dom.elementByTag(figure, 'figcaption');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import {Services} from '../../../src/services';
import {closestAncestorElementByTag, removeElement} from '../../../src/dom';
import {closestAncestorElementBySelector, removeElement} from '../../../src/dom';
import {dev, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {getMode} from '../../../src/mode';
Expand Down Expand Up @@ -268,7 +268,7 @@ class UrlRewriter_ {
return;
}
const target =
closestAncestorElementByTag(dev().assertElement(event.target), 'A');
closestAncestorElementBySelector(dev().assertElement(event.target), 'A');
if (!target || !target.href) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {Services} from '../../../../src/services';
import {
childElement,
childElementByAttr,
closestAncestorElementByTag,
closestAncestorElementBySelector,
elementByTag,
iterateCursor,
} from '../../../../src/dom';
Expand Down Expand Up @@ -303,7 +303,7 @@ export class LightboxManager {
getDescription(element) {
// If the element in question is the descendant of a figure element
// try using the figure caption as the lightbox description.
const figureParent = closestAncestorElementByTag(element, 'figure');
const figureParent = closestAncestorElementBySelector(element, 'figure');
if (figureParent) {
const figCaption = elementByTag(figureParent, 'figcaption');
if (figCaption) {
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-sidebar/0.1/amp-sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {CSS} from '../../../build/amp-sidebar-0.1.css';
import {Keys} from '../../../src/utils/key-codes';
import {Services} from '../../../src/services';
import {Toolbar} from './toolbar';
import {closestAncestorElementByTag, isRTL, tryFocus} from '../../../src/dom';
import {closestAncestorElementBySelector, isRTL, tryFocus} from '../../../src/dom';
import {createCustomEvent} from '../../../src/event-helper';
import {descendsFromStory} from '../../../src/utils/story';
import {dev} from '../../../src/log';
Expand Down Expand Up @@ -186,7 +186,7 @@ export class AmpSidebar extends AMP.BaseElement {

element.addEventListener('click', e => {
const target =
closestAncestorElementByTag(dev().assertElement(e.target), 'A');
closestAncestorElementBySelector(dev().assertElement(e.target), 'A');
if (target && target.href) {
const tgtLoc = Services.urlForDoc(element).parse(target.href);
const currentHref = this.getAmpDoc().win.location.href;
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-story/0.1/amp-story-consent.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {Services} from '../../../src/services';
import {assertAbsoluteHttpOrHttpsUrl} from '../../../src/url';
import {
childElementByTag,
closestAncestorElementByTag,
closestAncestorElementBySelector,
isJsonScriptTag,
} from '../../../src/dom';
import {computedStyle, setImportantStyles} from '../../../src/style';
Expand Down Expand Up @@ -197,8 +197,8 @@ export class AmpStoryConsent extends AMP.BaseElement {
buildCallback() {
this.assertAndParseConfig_();

const storyEl = closestAncestorElementByTag(this.element, 'AMP-STORY');
const consentEl = closestAncestorElementByTag(this.element, 'AMP-CONSENT');
const storyEl = closestAncestorElementBySelector(this.element, 'AMP-STORY');
const consentEl = closestAncestorElementBySelector(this.element, 'AMP-CONSENT');
const consentId = consentEl.id;
this.storeService_.dispatch(Action.SET_CONSENT_ID, consentId);

Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-story/1.0/amp-story-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {Layout} from '../../../src/layout';
import {assertHttpsUrl} from '../../../src/url';
import {
closest,
closestAncestorElementByTag,
closestAncestorElementBySelector,
copyChildren,
removeChildren,
} from '../../../src/dom';
Expand Down Expand Up @@ -243,7 +243,7 @@ export class AmpStoryAccess extends AMP.BaseElement {
*/
getLogoSrc_() {
const storyEl = dev().assertElement(
closestAncestorElementByTag(this.element, 'AMP-STORY'));
closestAncestorElementBySelector(this.element, 'AMP-STORY'));
const logoSrc = storyEl && storyEl.getAttribute('publisher-logo-src');

logoSrc ?
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-story/1.0/amp-story-consent.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {Services} from '../../../src/services';
import {assertAbsoluteHttpOrHttpsUrl, assertHttpsUrl} from '../../../src/url';
import {
childElementByTag,
closestAncestorElementByTag,
closestAncestorElementBySelector,
isJsonScriptTag,
} from '../../../src/dom';
import {computedStyle, setImportantStyles} from '../../../src/style';
Expand Down Expand Up @@ -201,8 +201,8 @@ export class AmpStoryConsent extends AMP.BaseElement {
this.assertAndParseConfig_();

const storyEl = dev().assertElement(
closestAncestorElementByTag(this.element, 'AMP-STORY'));
const consentEl = closestAncestorElementByTag(this.element, 'AMP-CONSENT');
closestAncestorElementBySelector(this.element, 'AMP-STORY'));
const consentEl = closestAncestorElementBySelector(this.element, 'AMP-CONSENT');
const consentId = consentEl.id;

this.storeConsentId_(consentId);
Expand Down
2 changes: 1 addition & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,7 @@ function createBaseCustomElementClass(win) {
connectedCallback() {
if (!isTemplateTagSupported() && this.isInTemplate_ === undefined) {
this.isInTemplate_ =
!!dom.closestAncestorElementByTag(this, 'template');
!!dom.closestAncestorElementBySelector(this, 'template');
}
if (this.isInTemplate_) {
return;
Expand Down
17 changes: 0 additions & 17 deletions src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,23 +259,6 @@ export function closestNode(node, callback) {
}


/**
* Finds the closest ancestor element with the specified name from this element
* up the DOM subtree.
* @param {!Element} element
* @param {string} tagName
* @return {?Element}
*/
export function closestAncestorElementByTag(element, tagName) {
if (element.closest) {
return element.closest(tagName);
}
tagName = tagName.toUpperCase();
return closest(element, el => {
return el.tagName == tagName;
});
}

/**
* Finds the closest ancestor element with the specified selector from this
* element.
Expand Down
6 changes: 3 additions & 3 deletions src/service/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import {Services} from '../services';
import {
closestAncestorElementByTag,
closestAncestorElementBySelector,
escapeCssSelectorIdent,
isIframed,
openWindowDialog,
Expand Down Expand Up @@ -311,7 +311,7 @@ export class Navigation {
return;
}
const element = dev().assertElement(e.target);
const target = closestAncestorElementByTag(element, 'A');
const target = closestAncestorElementBySelector(element, 'A');
if (!target || !target.href) {
return;
}
Expand Down Expand Up @@ -601,7 +601,7 @@ export class Navigation {
*/
function maybeExpandUrlParams(ampdoc, e) {
const target =
closestAncestorElementByTag(dev().assertElement(e.target), 'A');
closestAncestorElementBySelector(dev().assertElement(e.target), 'A');
if (!target || !target.href) {
// Not a click on a link.
return;
Expand Down
4 changes: 2 additions & 2 deletions src/utils/story.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {closestAncestorElementByTag, waitForChild} from '../dom';
import {closestAncestorElementBySelector, waitForChild} from '../dom';


/**
Expand All @@ -26,7 +26,7 @@ import {closestAncestorElementByTag, waitForChild} from '../dom';
* @return {boolean}
*/
export function descendsFromStory(element) {
return !!closestAncestorElementByTag(element, 'amp-story');
return !!closestAncestorElementBySelector(element, 'amp-story');
}

/**
Expand Down
10 changes: 5 additions & 5 deletions test/unit/test-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@ describes.sandboxed('DOM', {}, env => {

expect(dom.closest(child, () => true)).to.equal(child);
expect(dom.closestNode(child, () => true)).to.equal(child);
expect(dom.closestAncestorElementByTag(child, 'div')).to.equal(child);
expect(dom.closestAncestorElementByTag(child, 'DIV')).to.equal(child);
expect(dom.closestAncestorElementBySelector(child, 'div')).to.equal(child);
expect(dom.closestAncestorElementBySelector(child, 'DIV')).to.equal(child);
});

it('closest should stop search at opt_stopAt', () => {
Expand Down Expand Up @@ -223,16 +223,16 @@ describes.sandboxed('DOM', {}, env => {

expect(dom.closest(child, e => e.tagName == 'CHILD')).to.equal(child);
expect(dom.closestNode(child, e => e.tagName == 'CHILD')).to.equal(child);
expect(dom.closestAncestorElementByTag(child, 'child')).to.equal(child);
expect(dom.closestAncestorElementBySelector(child, 'child')).to.equal(child);

expect(dom.closest(child, e => e.tagName == 'ELEMENT')).to.equal(element);
expect(dom.closestNode(child, e => e.tagName == 'ELEMENT'))
.to.equal(element);
expect(dom.closestAncestorElementByTag(child, 'element')).to.equal(element);
expect(dom.closestAncestorElementBySelector(child, 'element')).to.equal(element);

expect(dom.closest(child, e => e.tagName == 'PARENT')).to.equal(parent);
expect(dom.closestNode(child, e => e.tagName == 'PARENT')).to.equal(parent);
expect(dom.closestAncestorElementByTag(child, 'parent')).to.equal(parent);
expect(dom.closestAncestorElementBySelector(child, 'parent')).to.equal(parent);
});

it('closestNode should find nodes as well as elements', () => {
Expand Down