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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor amp / bento classes #37156

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7b9c70a
extract defineBentoElement out of CustomElement class
kvchari Dec 7, 2021
d7ac0d5
create empty amp-pbe class
kvchari Dec 10, 2021
4974a8b
update 1.0 extension template
kvchari Dec 9, 2021
fdc57e7
adjust fixtures
kvchari Dec 7, 2021
3efafab
refactor amp/bento-accordion
kvchari Dec 10, 2021
31ce2c7
refactor amp/bento-base-carousel
kvchari Dec 10, 2021
2d567c9
refactor amp/bento-date-countdown
kvchari Dec 10, 2021
ef9f240
refactor amp/bento-date-display
kvchari Dec 10, 2021
8a00b1d
refactor amp/bento-embedly-card
kvchari Dec 10, 2021
5e6e3af
refactor amp/bento-facebook
kvchari Dec 10, 2021
2f96dc1
refactor amp/bento-fit-text
kvchari Dec 10, 2021
7b6f0d1
refactor amp/bento-iframe
kvchari Dec 10, 2021
2224414
refactor amp/bento-inline-gallery
kvchari Dec 10, 2021
8c2d595
refactor amp/bento-instagram
kvchari Dec 10, 2021
cb2326d
refactor amp/bento-lightbox
kvchari Dec 10, 2021
59494db
refactor amp/bento-mathml
kvchari Dec 10, 2021
b0d556a
refactor amp/bento-selector
kvchari Dec 10, 2021
258640b
refactor amp/bento-sidebar
kvchari Dec 10, 2021
6b2d622
refactor amp/bento-social-share
kvchari Dec 10, 2021
1cc1ae8
refactor amp/bento-soundcloud
kvchari Dec 10, 2021
f4c5e60
refactor amp/bento-stream-gallery
kvchari Dec 10, 2021
c71caae
refactor amp/bento-timeago
kvchari Dec 10, 2021
12526f0
refactor amp/bento-twitter
kvchari Dec 10, 2021
8874d6e
refactor amp/bento-wordpress-embed
kvchari Dec 10, 2021
ceebf3c
refactor amp/bento-lightbox-gallery
kvchari Dec 10, 2021
697c309
refactor amp/bento video extensions:
kvchari Dec 10, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions build-system/tasks/extension-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -741,13 +741,11 @@ async function getBentoBuildFilename(dir, name, mode, options) {
*/
function generateBentoEntryPointSource(name, toExport) {
return dedent(`
import {defineBentoElement} from '../../../../src/preact/bento-ce';
import {BaseElement} from '../base-element';

function defineElement() {
customElements.define(
__name__,
BaseElement.CustomElement(BaseElement)
);
defineBentoElement(__name__, BaseElement);
}

${toExport ? 'export {defineElement};' : 'defineElement();'}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {BaseElement} from './base-element';
import {PreactBaseElement} from '#preact/base-element';
__css_import__;
import {dict} from '#core/types/object';
import {isExperimentOn} from '#experiments';
Expand All @@ -7,7 +7,7 @@ import {userAssert} from '#utils/log';
/** @const {string} */
const TAG = 'amp-__component_name_hyphenated__';

class Amp__component_name_pascalcase__ extends BaseElement {
class Amp__component_name_pascalcase__ extends PreactBaseElement {
/** @override */
init() {
// __do_not_submit__: This is example code only.
Expand Down
8 changes: 4 additions & 4 deletions build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ exports.rules = [
'extensions/amp-carousel/0.2/amp-carousel.js->extensions/amp-base-carousel/0.1/child-layout-manager.js',
'extensions/amp-inline-gallery/0.1/amp-inline-gallery.js->extensions/amp-base-carousel/0.1/carousel-events.js',
'extensions/amp-inline-gallery/0.1/amp-inline-gallery-thumbnails.js->extensions/amp-base-carousel/0.1/carousel-events.js',
'extensions/amp-inline-gallery/1.0/base-element.js->extensions/amp-base-carousel/1.0/carousel-props.js',
'extensions/amp-inline-gallery/1.0/element.js->extensions/amp-base-carousel/1.0/carousel-props.js',
'extensions/amp-inline-gallery/1.0/pagination-base-element.js->extensions/amp-base-carousel/1.0/carousel-props.js',

'extensions/amp-inline-gallery/1.0/component.js->extensions/amp-base-carousel/1.0/carousel-context.js',
Expand All @@ -204,7 +204,7 @@ exports.rules = [
'extensions/amp-stream-gallery/0.1/amp-stream-gallery.js->extensions/amp-base-carousel/0.1/carousel-events.js',
'extensions/amp-stream-gallery/0.1/amp-stream-gallery.js->extensions/amp-base-carousel/0.1/child-layout-manager.js',
'extensions/amp-stream-gallery/0.1/amp-stream-gallery.js->extensions/amp-base-carousel/0.1/responsive-attributes.js',
'extensions/amp-stream-gallery/1.0/base-element.js->extensions/amp-base-carousel/1.0/component.jss.js',
'extensions/amp-stream-gallery/1.0/element.js->extensions/amp-base-carousel/1.0/component.jss.js',
'extensions/amp-stream-gallery/1.0/component.js->extensions/amp-base-carousel/1.0/component.js',

// <amp-dailymotion> versions share this message API definition.
Expand All @@ -214,8 +214,8 @@ exports.rules = [
'extensions/amp-base-carousel/1.0/scroller.js->extensions/amp-lightbox-gallery/1.0/component.js',
'extensions/amp-lightbox-gallery/1.0/provider.js->extensions/amp-lightbox/1.0/component.js',
'extensions/amp-lightbox-gallery/1.0/provider.js->extensions/amp-base-carousel/1.0/component.js',
'extensions/amp-lightbox-gallery/1.0/base-element.js->extensions/amp-lightbox/1.0/component.jss.js',
'extensions/amp-lightbox-gallery/1.0/base-element.js->extensions/amp-base-carousel/1.0/component.jss.js',
'extensions/amp-lightbox-gallery/1.0/element.js->extensions/amp-lightbox/1.0/component.jss.js',
'extensions/amp-lightbox-gallery/1.0/element.js->extensions/amp-base-carousel/1.0/component.jss.js',

// <amp-date-display> versions share these date format helpers
'extensions/amp-date-display/**->extensions/amp-date-display/format.js',
Expand Down
18 changes: 15 additions & 3 deletions extensions/amp-accordion/1.0/amp-accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,22 @@ import {getWin} from '#core/window';

import {isExperimentOn} from '#experiments';

import {AmpPreactBaseElement} from '#preact/amp-base-element';

import {Services} from '#service';

import {createCustomEvent} from '#utils/event-helper';
import {userAssert} from '#utils/log';

import {BaseElement} from './base-element';
import {Component, detached, elementInit, props} from './element';

import {CSS} from '../../../build/amp-accordion-1.0.css';

/** @const {string} */
const TAG = 'amp-accordion';

/** @extends {PreactBaseElement<BentoAccordionDef.AccordionApi>} */
class AmpAccordion extends BaseElement {
class AmpAccordion extends AmpPreactBaseElement {
/** @override */
init() {
this.registerApiAction('toggle', (api, invocation) =>
Expand All @@ -29,7 +31,11 @@ class AmpAccordion extends BaseElement {
api./*OK*/ collapse(invocation.args && invocation.args['section'])
);

return super.init();
return elementInit(
this.element,
this.mutateProps.bind(this),
this.triggerEvent.bind(this)
);
}

/** @override */
Expand Down Expand Up @@ -59,6 +65,12 @@ class AmpAccordion extends BaseElement {
return true;
}
}
/** @override */
AmpAccordion['Component'] = Component;
/** @override */
AmpAccordion['props'] = props;
/** @override */
AmpAccordion['detached'] = detached;

AMP.extension(TAG, '1.0', (AMP) => {
AMP.registerElement(TAG, AmpAccordion, CSS);
Expand Down
218 changes: 10 additions & 208 deletions extensions/amp-accordion/1.0/base-element.js
Original file line number Diff line number Diff line change
@@ -1,221 +1,23 @@
import {devAssert} from '#core/assert';
import {toggleAttribute} from '#core/dom';
import {childElementsByTag} from '#core/dom/query';
import {toArray} from '#core/types/array';
import {dict, memo} from '#core/types/object';
import {dispatchCustomEvent} from '#core/dom';

import * as Preact from '#preact';
import {useLayoutEffect, useRef} from '#preact';
import {PreactBaseElement} from '#preact/base-element';
import {forwardRef} from '#preact/compat';
import {useDOMHandle} from '#preact/component';
import {useSlotContext} from '#preact/slot';

import {
BentoAccordion,
BentoAccordionContent,
BentoAccordionHeader,
BentoAccordionSection,
} from './component';
import {Component, detached, elementInit, props} from './element';

const HEADER_SHIM_PROP = '__AMP_H_SHIM';
const CONTENT_SHIM_PROP = '__AMP_C_SHIM';
const SECTION_POST_RENDER = '__AMP_PR';
const EXPAND_STATE_SHIM_PROP = '__AMP_EXPAND_STATE_SHIM';

/** @extends {PreactBaseElement<BentoAccordionDef.AccordionApi>} */
/** @extends {PreactBaseElement<BentoAccordionDef.BentoAccordionApi>} */
export class BaseElement extends PreactBaseElement {
/** @override */
init() {
const getExpandStateTrigger = (section) => (expanded) => {
toggleAttribute(section, 'expanded', expanded);
section[SECTION_POST_RENDER]?.();
this.triggerEvent(section, expanded ? 'expand' : 'collapse');
};

const {element} = this;
const mu = new MutationObserver(() => {
this.mutateProps(getState(element, mu, getExpandStateTrigger));
});
mu.observe(element, {
attributeFilter: ['expanded', 'id'],
subtree: true,
childList: true,
});

const {'children': children} = getState(element, mu, getExpandStateTrigger);
return dict({
'children': children,
});
}
}

/**
* @param {!Element} element
* @param {MutationObserver} mu
* @param {!Function} getExpandStateTrigger
* @return {!JsonObject}
*/
function getState(element, mu, getExpandStateTrigger) {
const sections = toArray(childElementsByTag(element, 'section'));

const children = sections.map((section) => {
// TODO(wg-bento): This implementation causes infinite loops on DOM mutation.
// See https://github.com/ampproject/amp-react-prototype/issues/40.
if (!section[SECTION_POST_RENDER]) {
section[SECTION_POST_RENDER] = () => mu.takeRecords();
}

const headerShim = memo(section, HEADER_SHIM_PROP, bindHeaderShimToElement);
const contentShim = memo(
section,
CONTENT_SHIM_PROP,
bindContentShimToElement
return elementInit(
this.element,
this.mutateProps.bind(this), // eslint-disable-line local/restrict-this-access
dispatchCustomEvent
);
const expandStateShim = memo(
section,
EXPAND_STATE_SHIM_PROP,
getExpandStateTrigger
);
const sectionProps = dict({
'key': section,
'expanded': section.hasAttribute('expanded'),
'id': section.getAttribute('id'),
'onExpandStateChange': expandStateShim,
});
// For headerProps and contentProps:
// || undefined needed for the `role` attribute since an element w/o
// role results in `null`. When `null` is passed into Preact, the
// default prop value is not used. `undefined` triggers default prop
// value. This is not needed for `id` since this is handled with
// explicit logic (not default prop value) and all falsy values are
// handled the same.
const headerProps = dict({
'as': headerShim,
'id': section.firstElementChild.getAttribute('id'),
'role': section.firstElementChild.getAttribute('role') || undefined,
});
const contentProps = dict({
'as': contentShim,
'id': section.lastElementChild.getAttribute('id'),
'role': section.lastElementChild.getAttribute('role') || undefined,
});
return (
<BentoAccordionSection {...sectionProps}>
<BentoAccordionHeader {...headerProps}></BentoAccordionHeader>
<BentoAccordionContent {...contentProps}></BentoAccordionContent>
</BentoAccordionSection>
);
});
return dict({'children': children});
}

/**
* @param {!Element} sectionElement
* @param {!BentoAccordionDef.HeaderShimProps} props
* @return {PreactDef.Renderable}
*/
function HeaderShim(
sectionElement,
{
'aria-controls': ariaControls,
'aria-expanded': ariaExpanded,
id,
onClick,
role,
}
) {
const headerElement = sectionElement.firstElementChild;
useLayoutEffect(() => {
if (!headerElement || !onClick) {
return;
}
headerElement.setAttribute('id', id);
headerElement.classList.add('i-amphtml-accordion-header');
headerElement.addEventListener('click', onClick);
if (!headerElement.hasAttribute('tabindex')) {
headerElement.setAttribute('tabindex', 0);
}
headerElement.setAttribute('aria-expanded', ariaExpanded);
headerElement.setAttribute('aria-controls', ariaControls);
headerElement.setAttribute('role', role);
if (sectionElement[SECTION_POST_RENDER]) {
sectionElement[SECTION_POST_RENDER]();
}
return () => {
headerElement.removeEventListener('click', devAssert(onClick));
};
}, [
sectionElement,
headerElement,
id,
role,
onClick,
ariaControls,
ariaExpanded,
]);
return <header />;
}

/**
* @param {!Element} element
* @return {function(!BentoAccordionDef.HeaderProps):PreactDef.Renderable}
*/
const bindHeaderShimToElement = (element) => HeaderShim.bind(null, element);

/**
* @param {!Element} sectionElement
* @param {!BentoAccordionDef.ContentShimProps} props
* @param {{current: ?}} ref
* @return {PreactDef.Renderable}
*/
function ContentShimWithRef(
sectionElement,
{'aria-labelledby': ariaLabelledBy, id, role},
ref
) {
const contentElement = sectionElement.lastElementChild;
const contentRef = useRef();
contentRef.current = contentElement;
useSlotContext(contentRef);
useDOMHandle(ref, contentElement);
useLayoutEffect(() => {
if (!contentElement) {
return;
}
contentElement.classList.add('i-amphtml-accordion-content');
contentElement.setAttribute('id', id);
contentElement.setAttribute('role', role);
contentElement.setAttribute('aria-labelledby', ariaLabelledBy);
if (sectionElement[SECTION_POST_RENDER]) {
sectionElement[SECTION_POST_RENDER]();
}
}, [sectionElement, contentElement, id, role, ariaLabelledBy]);
return <div />;
}

/**
* @param {!Element} element
* @return {function(!BentoAccordionDef.ContentProps):PreactDef.Renderable}
*/
const bindContentShimToElement = (element) =>
forwardRef(
/** @type {function(?, {current:?}):PreactDef.Renderable} */ (
ContentShimWithRef.bind(null, element)
)
);

/** @override */
BaseElement['Component'] = BentoAccordion;
BaseElement['Component'] = Component;

/** @override */
BaseElement['detached'] = true;
BaseElement['props'] = props;

/** @override */
BaseElement['props'] = {
'animate': {attr: 'animate', type: 'boolean', media: true},
'expandSingleSection': {
attr: 'expand-single-section',
type: 'boolean',
},
};
BaseElement['detached'] = detached;
Loading