more comments addressed

microsoft · May 7, 2020 · 5a2cc98 · 5a2cc98
1 parent f523846
commit 5a2cc98
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 65 deletions.
diff --git a/...ages/web-components/fast-components/src/accordion/accordion-item/accordion-item.styles.ts b/...ages/web-components/fast-components/src/accordion/accordion-item/accordion-item.styles.ts
@@ -17,7 +17,6 @@ export const AccordionItemStyles = css`
         font-family: var(--body-font);
         flex-direction: column;
         font-size: var(--type-ramp-minus-1-font-size);
-        font-weight: 400;
         line-height: var(--type-ramp-minus-1-line-height);
         border-bottom: calc(var(--outline-width) * 1px) solid var(--neutral-divider-rest);
     }
@@ -30,7 +29,8 @@ export const AccordionItemStyles = css`
     .heading {
         display: grid;
         position: relative;
-        grid-template-columns: auto 1fr auto calc(${heightNumber} * 1px)
+        grid-template-columns: auto 1fr auto calc(${heightNumber} * 1px);
+        z-index: 2;
     }
 
     .button {
@@ -77,10 +77,6 @@ export const AccordionItemStyles = css`
         display: flex;
     }
 
-    .heading {
-        z-index: 2;
-    }
-
     .icon {
         display: flex;
         align-items: center;

diff --git a/...es/web-components/fast-components/src/accordion/accordion-item/accordion-item.template.ts b/...es/web-components/fast-components/src/accordion/accordion-item/accordion-item.template.ts
@@ -17,7 +17,7 @@ export const AccordionItemTemplate = html<AccordionItem>`
             <button
                 class="button"
                 part="button"
-                ${ref("button")}
+                ${ref("expandbutton")}
                 aria-expanded="${x => x.expanded}"
                 aria-controls="${x => x.id}-panel"
                 id="${x => x.id}"

diff --git a/packages/web-components/fast-components/src/accordion/accordion-item/accordion-item.ts b/packages/web-components/fast-components/src/accordion/accordion-item/accordion-item.ts
@@ -1,17 +1,8 @@
-import { attr, FASTElement, observable } from "@microsoft/fast-element";
-
-export enum AccordionHeadingLevel {
-    h1 = "1",
-    h2 = "2",
-    h3 = "3",
-    h4 = "4",
-    h5 = "5",
-    h6 = "6",
-}
+import { attr, booleanConverter, FASTElement, observable } from "@microsoft/fast-element";
 
 export class AccordionItem extends FASTElement {
-    @attr
-    public headinglevel: AccordionHeadingLevel = AccordionHeadingLevel.h2;
+    @attr({ attribute: "heading-level", mode: "fromView", converter: booleanConverter })
+    public headinglevel: 1 | 2 | 3 | 4 | 5 | 6 = 2;
 
     @attr({ mode: "boolean" })
     public expanded: boolean = false;
@@ -20,7 +11,7 @@ export class AccordionItem extends FASTElement {
     public id: string;
 
     @observable
-    public button: HTMLElement;
+    public expandbutton: HTMLElement;
 
     /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
     public clickHandler = (e: MouseEvent) => {
@@ -29,6 +20,6 @@ export class AccordionItem extends FASTElement {
     };
 
     private change = (): void => {
-        this.$emit("change", this.id);
+        this.$emit("change");
     };
 }
diff --git a/packages/web-components/fast-components/src/accordion/accordion.styles.ts b/packages/web-components/fast-components/src/accordion/accordion.styles.ts
@@ -11,7 +11,6 @@ export const AccordionStyles = css`
         flex-direction: column;
         font-family: var(--body-font);
         font-size: var(--type-ramp-minus-1-font-size);
-        font-weight: 400;
         line-height: var(--type-ramp-minus-1-line-height);
         color: var(--neutral-foreground-rest);
         border-top: calc(var(--outline-width) * 1px) solid var(--neutral-divider-rest);

diff --git a/packages/web-components/fast-components/src/accordion/accordion.ts b/packages/web-components/fast-components/src/accordion/accordion.ts
@@ -1,4 +1,4 @@
-import { attr, FASTElement, observable } from "@microsoft/fast-element";
+import { attr, booleanConverter, FASTElement, observable } from "@microsoft/fast-element";
 import {
     keyCodeArrowDown,
     keyCodeArrowUp,
@@ -16,73 +16,75 @@ export enum AccordionExpandMode {
 }
 
 export class Accordion extends FASTElement {
-    @attr
+    @attr({ attribute: "expand-mode" })
     public expandmode: AccordionExpandMode = AccordionExpandMode.multi;
 
     @observable
     public accordionItems: HTMLElement[];
     public accordionItemsChanged(oldValue, newValue): void {
         if (this.$fastController.isConnected) {
-            this.removeHeaderListeners(oldValue);
-            this.accordionIds = this.getHeaderIds();
+            this.removeItemListeners(oldValue);
+            this.accordionIds = this.getItemIds();
             this.setItems();
         }
     }
 
     private activeid: string;
-    private activeHeaderIndex: number = 0;
+    private activeItemIndex: number = 0;
     private accordionIds: Array<string | null>;
 
     private change = (): void => {
-        this.$emit("change", this.activeid);
+        this.$emit("change");
     };
 
     private setItems = (): void => {
-        this.accordionIds = this.getHeaderIds();
-        this.accordionItems.forEach((header: HTMLElement, index: number) => {
-            if (header instanceof FASTAccordionItem) {
-                header.addEventListener("change", this.activeHeaderChange);
+        console.log(this.isSingleExpandMode());
+        this.accordionIds = this.getItemIds();
+        this.accordionItems.forEach((item: HTMLElement, index: number) => {
+            if (item instanceof FASTAccordionItem) {
+                item.addEventListener("change", this.activeItemChange);
                 if (this.isSingleExpandMode()) {
-                    this.activeHeaderIndex !== index
-                        ? (header.expanded = false)
-                        : (header.expanded = true);
+                    this.activeItemIndex !== index
+                        ? (item.expanded = false)
+                        : (item.expanded = true);
                 }
             }
-            const headerId: string | null = this.accordionIds[index];
-            header.setAttribute(
+            const itemId: string | null = this.accordionIds[index];
+            item.setAttribute(
                 "id",
-                typeof headerId !== "string" ? `accordion-${index + 1}` : headerId
+                typeof itemId !== "string" ? `accordion-${index + 1}` : itemId
             );
-            this.activeid = this.accordionIds[this.activeHeaderIndex] as string;
-            header.addEventListener("keydown", this.handleHeaderKeyDown);
+            this.activeid = this.accordionIds[this.activeItemIndex] as string;
+            item.addEventListener("keydown", this.handleItemKeyDown);
         });
     };
 
-    private resetHeaders = (): void => {
-        this.accordionItems.forEach((header: FASTAccordionItem, index: number) => {
-            header.expanded = false;
+    private resetItems = (): void => {
+        this.accordionItems.forEach((item: FASTAccordionItem, index: number) => {
+            item.expanded = false;
         });
     };
 
-    private removeHeaderListeners = (oldValue: any): void => {
-        oldValue.forEach((header: HTMLElement, index: number) => {
-            header.removeEventListener("change", this.activeHeaderChange);
-            header.removeEventListener("keydown", this.handleHeaderKeyDown);
+    private removeItemListeners = (oldValue: any): void => {
+        oldValue.forEach((item: HTMLElement, index: number) => {
+            item.removeEventListener("change", this.activeItemChange);
+            item.removeEventListener("keydown", this.handleItemKeyDown);
         });
     };
 
-    private activeHeaderChange = (event): void => {
-        const selectedHeader = event.target as HTMLElement;
+    private activeItemChange = (event): void => {
+        const selectedItem = event.target as HTMLElement;
         if (this.isSingleExpandMode()) {
-            this.resetHeaders();
+            console.log("Hits");
+            this.resetItems();
             event.target.expanded = true;
         }
         this.activeid = event.target.getAttribute("id");
-        this.activeHeaderIndex = Array.from(this.accordionItems).indexOf(selectedHeader);
+        this.activeItemIndex = Array.from(this.accordionItems).indexOf(selectedItem);
         this.change();
     };
 
-    private getHeaderIds(): Array<string | null> {
+    private getItemIds(): Array<string | null> {
         return this.accordionItems.map((accordionItem: HTMLElement) => {
             return accordionItem.getAttribute("id");
         });
@@ -92,9 +94,9 @@ export class Accordion extends FASTElement {
         return this.expandmode === AccordionExpandMode.single;
     }
 
-    private handleHeaderKeyDown = (event: KeyboardEvent): void => {
+    private handleItemKeyDown = (event: KeyboardEvent): void => {
         const keyCode: number = event.keyCode;
-        this.accordionIds = this.getHeaderIds();
+        this.accordionIds = this.getItemIds();
         switch (keyCode) {
             case keyCodeArrowUp:
                 event.preventDefault();
@@ -105,29 +107,29 @@ export class Accordion extends FASTElement {
                 this.adjust(1);
                 break;
             case keyCodeHome:
-                this.activeHeaderIndex = 0;
-                this.focusHeader();
+                this.activeItemIndex = 0;
+                this.focusItem();
                 break;
             case keyCodeEnd:
-                this.activeHeaderIndex = this.accordionItems.length - 1;
-                this.focusHeader();
+                this.activeItemIndex = this.accordionItems.length - 1;
+                this.focusItem();
                 break;
         }
     };
 
     private adjust(adjustment: number): void {
-        this.activeHeaderIndex = wrapInBounds(
+        this.activeItemIndex = wrapInBounds(
             0,
             this.accordionItems.length - 1,
-            this.activeHeaderIndex + adjustment
+            this.activeItemIndex + adjustment
         );
-        this.focusHeader();
+        this.focusItem();
     }
 
-    private focusHeader(): void {
-        const element: HTMLElement = this.accordionItems[this.activeHeaderIndex];
+    private focusItem(): void {
+        const element: HTMLElement = this.accordionItems[this.activeItemIndex];
         if (element instanceof FASTAccordionItem) {
-            element.button.focus();
+            element.expandbutton.focus();
         }
     }
 }

diff --git a/packages/web-components/fast-components/src/accordion/fixtures/base.html b/packages/web-components/fast-components/src/accordion/fixtures/base.html
@@ -49,7 +49,7 @@ <h4>Default</h4>
         </fast-accordion-item>
     </fast-accordion>
     <h4>Single expand</h4>
-    <fast-accordion expandmode="single">
+    <fast-accordion expand-mode="single">
         <fast-accordion-item>
             <div slot="start">
                 <button>1</button>