✨ [bento][amp-accordion] CSS Port draft for amp accordion (PR ampproj…

…ect#2) (ampproject#30622)

* Update CSS for accordion

* Add JSX into Preact component styling and misc review comments

* Add important to make section styles impossible to overwrite

* Add tests for CSS in amp-accordion

* Accordion tests updates

* Removed tests for getComputedStyle

* Remove debugging comments

* Address nits from review

extensions/amp-accordion/1.0/accordion.js

@@ -30,6 +30,7 @@ import {
   useRef,
   useState,
 } from '../../../src/preact';
+import {useStyles} from './accordion.jss';
 
 const AccordionContext = Preact.createContext(
   /** @type {AccordionDef.ContextProps} */ ({})
@@ -41,17 +42,6 @@ const EMPTY_EXPANDED_MAP = {};
 const generateSectionId = sequentialIdGenerator();
 const generateRandomId = randomIdGenerator(100000);
 
-const CHILD_STYLE = {
-  // Make animations measurable. Without this, padding and margin can skew
-  // animations.
-  boxSizing: 'border-box',
-  // Cancel out the margin collapse. Also helps with animations to avoid
-  // overflow.
-  overflow: 'hidden',
-  // Ensure that any absolute elements are positioned within the section.
-  position: 'relative',
-};
-
 /**
  * @param {!AccordionDef.Props} props
  * @return {PreactDef.Renderable}
@@ -160,6 +150,8 @@ export function AccordionSection({
   contentAs: ContentComp = 'div',
   expanded: defaultExpanded = false,
   animate: defaultAnimate = false,
+  headerClassName = '',
+  contentClassName = '',
   header,
   children,
   ...rest
@@ -200,6 +192,7 @@ export function AccordionSection({
   const expanded = isExpanded ? isExpanded(id, defaultExpanded) : expandedState;
   const animate = contextAnimate ?? defaultAnimate;
   const contentId = `${prefix || 'a'}-content-${id}-${suffix}`;
+  const classes = useStyles();
 
   useLayoutEffect(() => {
     const hasMounted = hasMountedRef.current;
@@ -214,17 +207,17 @@ export function AccordionSection({
     <Comp {...rest} expanded={expanded} aria-expanded={String(expanded)}>
       <HeaderComp
         role="button"
+        className={`${headerClassName} ${classes.sectionChild} ${classes.header}`}
         aria-controls={contentId}
         tabIndex="0"
-        style={CHILD_STYLE}
         onClick={expandHandler}
       >
         {header}
       </HeaderComp>
       <ContentComp
-        id={contentId}
         ref={contentRef}
-        style={CHILD_STYLE}
+        className={`${contentClassName} ${classes.sectionChild} ${classes.content}`}
+        id={contentId}
         hidden={!expanded}
       >
         {children}

extensions/amp-accordion/1.0/accordion.jss.js

@@ -0,0 +1,50 @@
+/**
+ * Copyright 2020 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {createUseStyles} from 'react-jss';
+
+const sectionChild = {
+  // Make animations measurable. Without this, padding and margin can skew
+  // animations.
+  boxSizing: 'border-box !important',
+  // Cancel out the margin collapse. Also helps with animations to avoid
+  // overflow.
+  overflow: 'hidden !important',
+  // Ensure that any absolute elements are positioned within the section.
+  position: 'relative !important',
+};
+
+// TODO(#30445): update these styles after team agrees on styling
+const header = {
+  cursor: 'pointer',
+  backgroundColor: '#efefef',
+  paddingRight: '20px',
+  border: 'solid 1px #dfdfdf',
+};
+
+// TODO(#30445): update these styles after team agrees on styling
+// or delete if not used
+const content = {};
+
+const JSS = {
+  sectionChild,
+  header,
+  content,
+};
+
+// useStyles gets replaced for AMP builds via `babel-plugin-transform-jss`.
+// eslint-disable-next-line local/no-export-side-effect
+export const useStyles = createUseStyles(JSS);

extensions/amp-accordion/1.0/accordion.type.js

@@ -36,6 +36,8 @@ AccordionDef.Props;
  *   contentAs: (string|PreactDef.FunctionalComponent|undefined),
  *   expanded: (boolean|undefined),
  *   animate: (boolean|undefined),
+ *   headerClassName: (string|undefined),
+ *   contentClassName: (string|undefined),
  *   header: (!PreactDef.Renderable),
  *   children: (?PreactDef.Renderable|undefined),
  * }}

extensions/amp-accordion/1.0/amp-accordion.css

@@ -13,3 +13,19 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+/* heading/content */
+.i-amphtml-accordion-header,
+.i-amphtml-accordion-content {
+  margin: 0;
+}
+
+/* heading
+ * TODO(#30445): update these styles after team agrees on styling
+ */
+.i-amphtml-accordion-header {
+  cursor: pointer;
+  background-color: #efefef;
+  padding-right: 20px;
+  border: solid 1px #dfdfdf;
+}

extensions/amp-accordion/1.0/amp-accordion.js

@@ -16,6 +16,7 @@
 
 import * as Preact from '../../../src/preact';
 import {Accordion, AccordionSection} from './accordion';
+import {CSS} from '../../../build/amp-accordion-1.0.css';
 import {PreactBaseElement} from '../../../src/preact/base-element';
 import {childElementsByTag, toggleAttribute} from '../../../src/dom';
 import {devAssert, userAssert} from '../../../src/log';
@@ -132,6 +133,7 @@ function HeaderShim(sectionElement, {onClick, 'aria-controls': ariaControls}) {
     if (!headerElement || !onClick) {
       return;
     }
+    headerElement.classList.add('i-amphtml-accordion-header');
     headerElement.addEventListener('click', onClick);
     if (!headerElement.hasAttribute('tabindex')) {
       headerElement.setAttribute('tabindex', 0);
@@ -167,6 +169,7 @@ function ContentShimWithRef(sectionElement, {hidden, id}, ref) {
     if (!contentElement) {
       return;
     }
+    contentElement.classList.add('i-amphtml-accordion-content');
     contentElement.setAttribute('id', id);
     toggleAttribute(contentElement, 'hidden', hidden);
     if (sectionElement[SECTION_POST_RENDER]) {
@@ -201,5 +204,5 @@ AmpAccordion['props'] = {
 };
 
 AMP.extension(TAG, '1.0', (AMP) => {
-  AMP.registerElement(TAG, AmpAccordion);
+  AMP.registerElement(TAG, AmpAccordion, CSS);
 });

extensions/amp-accordion/1.0/test/test-accordion.js

@@ -19,6 +19,7 @@ import {Accordion, AccordionSection} from '../accordion';
 import {mount} from 'enzyme';
 
 describes.sandboxed('Accordion preact component', {}, (env) => {
+  let win;
   describe('standalone accordion section', () => {
     it('should render a default section', () => {
       const wrapper = mount(
@@ -92,6 +93,7 @@ describes.sandboxed('Accordion preact component', {}, (env) => {
     let wrapper;
 
     beforeEach(() => {
+      win = env.win;
       wrapper = mount(
         <Accordion>
           <AccordionSection key={1} expanded header="header1">
@@ -103,7 +105,8 @@ describes.sandboxed('Accordion preact component', {}, (env) => {
           <AccordionSection key={3} header="header3">
             content3
           </AccordionSection>
-        </Accordion>
+        </Accordion>,
+        {attachTo: win}
       );
     });
 
@@ -119,18 +122,39 @@ describes.sandboxed('Accordion preact component', {}, (env) => {
       expect(sections.at(1).getDOMNode()).to.not.have.attribute('expanded');
       expect(sections.at(2).getDOMNode()).to.not.have.attribute('expanded');
 
+      const header0 = sections.at(0).find('header').getDOMNode();
+      const header1 = sections.at(1).find('header').getDOMNode();
+      const header2 = sections.at(2).find('header').getDOMNode();
+      const content0 = sections.at(0).find('div').getDOMNode();
+      const content1 = sections.at(1).find('div').getDOMNode();
+      const content2 = sections.at(2).find('div').getDOMNode();
+
       // Headers.
-      expect(sections.at(0).find('header').text()).to.equal('header1');
-      expect(sections.at(1).find('header').text()).to.equal('header2');
-      expect(sections.at(2).find('header').text()).to.equal('header3');
+      expect(header0.textContent).to.equal('header1');
+      expect(header1.textContent).to.equal('header2');
+      expect(header2.textContent).to.equal('header3');
 
       // Contents.
-      expect(sections.at(0).find('div').text()).to.equal('content1');
-      expect(sections.at(1).find('div').text()).to.equal('content2');
-      expect(sections.at(2).find('div').text()).to.equal('content3');
-      expect(sections.at(0).find('div').getDOMNode().hidden).to.be.false;
-      expect(sections.at(1).find('div').getDOMNode().hidden).to.be.true;
-      expect(sections.at(2).find('div').getDOMNode().hidden).to.be.true;
+      expect(content0.textContent).to.equal('content1');
+      expect(content1.textContent).to.equal('content2');
+      expect(content2.textContent).to.equal('content3');
+      expect(content0.hidden).to.be.false;
+      expect(content1.hidden).to.be.true;
+      expect(content2.hidden).to.be.true;
+
+      // Styling.
+      expect(header0.className.includes('section-child')).to.be.true;
+      expect(header0.className.includes('header')).to.be.true;
+      expect(header1.className.includes('section-child')).to.be.true;
+      expect(header1.className.includes('header')).to.be.true;
+      expect(header2.className.includes('section-child')).to.be.true;
+      expect(header2.className.includes('header')).to.be.true;
+      expect(content0.className.includes('section-child')).to.be.true;
+      expect(content0.className.includes('content')).to.be.true;
+      expect(content1.className.includes('section-child')).to.be.true;
+      expect(content1.className.includes('content')).to.be.true;
+      expect(content2.className.includes('section-child')).to.be.true;
+      expect(content2.className.includes('content')).to.be.true;
     });
 
     it('should include a11y related attributes', () => {

extensions/amp-accordion/1.0/test/test-amp-accordion.js

@@ -76,6 +76,65 @@ describes.realWin(
       expect(sections[2].lastElementChild).to.have.display('none');
     });
 
+    it('should have amp specific classes for CSS', () => {
+      const sections = element.children;
+      const {
+        firstElementChild: header0,
+        lastElementChild: content0,
+      } = sections[0];
+      const {
+        firstElementChild: header1,
+        lastElementChild: content1,
+      } = sections[1];
+      const {
+        firstElementChild: header2,
+        lastElementChild: content2,
+      } = sections[2];
+
+      // Check classes
+      expect(header0.className).to.include('i-amphtml-accordion-header');
+      expect(header1.className).to.include('i-amphtml-accordion-header');
+      expect(header2.className).to.include('i-amphtml-accordion-header');
+      expect(content0.className).to.include('i-amphtml-accordion-content');
+      expect(content1.className).to.include('i-amphtml-accordion-content');
+      expect(content2.className).to.include('i-amphtml-accordion-content');
+
+      // Check computed styles
+      expect(win.getComputedStyle(header0).margin).to.equal('0px');
+      expect(win.getComputedStyle(header0).cursor).to.equal('pointer');
+      expect(win.getComputedStyle(header0).backgroundColor).to.equal(
+        'rgb(239, 239, 239)'
+      );
+      expect(win.getComputedStyle(header0).paddingRight).to.equal('20px');
+      expect(win.getComputedStyle(header0).border).to.equal(
+        '1px solid rgb(223, 223, 223)'
+      );
+
+      expect(win.getComputedStyle(header1).margin).to.equal('0px');
+      expect(win.getComputedStyle(header1).cursor).to.equal('pointer');
+      expect(win.getComputedStyle(header1).backgroundColor).to.equal(
+        'rgb(239, 239, 239)'
+      );
+      expect(win.getComputedStyle(header1).paddingRight).to.equal('20px');
+      expect(win.getComputedStyle(header1).border).to.equal(
+        '1px solid rgb(223, 223, 223)'
+      );
+
+      expect(win.getComputedStyle(header2).margin).to.equal('0px');
+      expect(win.getComputedStyle(header2).cursor).to.equal('pointer');
+      expect(win.getComputedStyle(header2).backgroundColor).to.equal(
+        'rgb(239, 239, 239)'
+      );
+      expect(win.getComputedStyle(header2).paddingRight).to.equal('20px');
+      expect(win.getComputedStyle(header2).border).to.equal(
+        '1px solid rgb(223, 223, 223)'
+      );
+
+      expect(win.getComputedStyle(content0).margin).to.equal('0px');
+      expect(win.getComputedStyle(content1).margin).to.equal('0px');
+      expect(win.getComputedStyle(content2).margin).to.equal('0px');
+    });
+
     it('should expand and collapse on click', async () => {
       const sections = element.children;