Bento Accordion: Remove unrecognized expanded DOM attribute (#36169)

* Do not propagate `expanded` prop * Remove unnecessary section * Fix test
ampproject · Oct 5, 2021 · dcaeff1 · dcaeff1
1 parent a690bd4
commit dcaeff1
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 115 deletions.
diff --git a/extensions/amp-accordion/1.0/base-element.js b/extensions/amp-accordion/1.0/base-element.js
@@ -19,7 +19,6 @@ import {
 
 import {devAssert} from '../../../src/log';
 
-const SECTION_SHIM_PROP = '__AMP_S_SHIM';
 const HEADER_SHIM_PROP = '__AMP_H_SHIM';
 const CONTENT_SHIM_PROP = '__AMP_C_SHIM';
 const SECTION_POST_RENDER = '__AMP_PR';
@@ -29,8 +28,11 @@ const EXPAND_STATE_SHIM_PROP = '__AMP_EXPAND_STATE_SHIM';
 export class BaseElement extends PreactBaseElement {
   /** @override */
   init() {
-    const getExpandStateTrigger = (section) => (expanded) =>
+    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(() => {
@@ -65,11 +67,6 @@ function getState(element, mu, getExpandStateTrigger) {
       section[SECTION_POST_RENDER] = () => mu.takeRecords();
     }
 
-    const sectionShim = memo(
-      section,
-      SECTION_SHIM_PROP,
-      bindSectionShimToElement
-    );
     const headerShim = memo(section, HEADER_SHIM_PROP, bindHeaderShimToElement);
     const contentShim = memo(
       section,
@@ -83,7 +80,6 @@ function getState(element, mu, getExpandStateTrigger) {
     );
     const sectionProps = dict({
       'key': section,
-      'as': sectionShim,
       'expanded': section.hasAttribute('expanded'),
       'id': section.getAttribute('id'),
       'onExpandStateChange': expandStateShim,
@@ -115,27 +111,6 @@ function getState(element, mu, getExpandStateTrigger) {
   return dict({'children': children});
 }
 
-/**
- * @param {!Element} sectionElement
- * @param {!BentoAccordionDef.SectionShimProps} props
- * @return {PreactDef.Renderable}
- */
-function SectionShim(sectionElement, {children, expanded}) {
-  useLayoutEffect(() => {
-    toggleAttribute(sectionElement, 'expanded', expanded);
-    if (sectionElement[SECTION_POST_RENDER]) {
-      sectionElement[SECTION_POST_RENDER]();
-    }
-  }, [sectionElement, expanded]);
-  return children;
-}
-
-/**
- * @param {!Element} element
- * @return {function(!BentoAccordionDef.SectionProps):PreactDef.Renderable}
- */
-const bindSectionShimToElement = (element) => SectionShim.bind(null, element);
-
 /**
  * @param {!Element} sectionElement
  * @param {!BentoAccordionDef.HeaderShimProps} props

diff --git a/extensions/amp-accordion/1.0/component.js b/extensions/amp-accordion/1.0/component.js
@@ -324,7 +324,7 @@ export function BentoAccordionSection({
   );
 
   return (
-    <Comp {...rest} expanded={expanded}>
+    <Comp {...rest}>
       <SectionContext.Provider value={context}>
         {children}
       </SectionContext.Provider>

diff --git a/extensions/amp-accordion/1.0/test/test-amp-accordion.js b/extensions/amp-accordion/1.0/test/test-amp-accordion.js
@@ -184,7 +184,7 @@ describes.realWin(
       await waitForExpanded(sections[1], true);
 
       const getExpandedCount = () =>
-        element.querySelectorAll('[expanded]').length;
+        element.querySelectorAll('[aria-expanded="true"]').length;
       expect(getExpandedCount()).to.equal(2);
 
       element.setAttribute('expand-single-section', '');