feat(accordion)!: addressing review comments

adobe · Jun 26, 2023 · 84d86fe · 84d86fe
1 parent 8cfd28b
commit 84d86fe
Show file tree

Hide file tree

Showing 6 changed files with 187 additions and 12 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -10,7 +10,7 @@ executors:
 parameters:
     current_golden_images_hash:
         type: string
-        default: e17c6b8176c53977497a922618b67b373e7fcc47
+        default: 8eda90e60eb073d6b78741e5b3457324ca155d59
     wireit_cache_name:
         type: string
         default: wireit

diff --git a/packages/accordion/README.md b/packages/accordion/README.md
@@ -170,8 +170,33 @@ The `density` property, when applied, accepts the values of `compact` or `spacio
 
 ### Compact
 
+<sp-tabs selected="m" auto label="Size Attribute Options">
+<sp-tab value="s">Small</sp-tab>
+<sp-tab-panel value="s">
+
+```html
+<sp-accordion density="compact" size="s">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+<sp-tab value="m">Medium</sp-tab>
+<sp-tab-panel value="m">
+
 ```html
-<sp-accordion density="compact">
+<sp-accordion density="compact" size="m">
     <sp-accordion-item label="Heading 1">
         <div>Item 1</div>
     </sp-accordion-item>
@@ -187,10 +212,80 @@ The `density` property, when applied, accepts the values of `compact` or `spacio
 </sp-accordion>
 ```
 
+</sp-tab-panel>
+<sp-tab value="l">Large</sp-tab>
+<sp-tab-panel value="l">
+
+```html
+<sp-accordion density="compact" size="l">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+<sp-tab value="xl">Extra Large</sp-tab>
+<sp-tab-panel value="xl">
+
+```html
+<sp-accordion density="compact" size="xl">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+</sp-tabs>
+
 ### Spacious
 
+<sp-tabs selected="m" auto label="Size Attribute Options">
+<sp-tab value="s">Small</sp-tab>
+<sp-tab-panel value="s">
+
+```html
+<sp-accordion density="spacious" size="s">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+<sp-tab value="m">Medium</sp-tab>
+<sp-tab-panel value="m">
+
 ```html
-<sp-accordion density="spacious">
+<sp-accordion density="spacious" size="m">
     <sp-accordion-item label="Heading 1">
         <div>Item 1</div>
     </sp-accordion-item>
@@ -205,3 +300,48 @@ The `density` property, when applied, accepts the values of `compact` or `spacio
     </sp-accordion-item>
 </sp-accordion>
 ```
+
+</sp-tab-panel>
+<sp-tab value="l">Large</sp-tab>
+<sp-tab-panel value="l">
+
+```html
+<sp-accordion density="spacious" size="l">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+<sp-tab value="xl">Extra Large</sp-tab>
+<sp-tab-panel value="xl">
+
+```html
+<sp-accordion density="spacious" size="xl">
+    <sp-accordion-item label="Heading 1">
+        <div>Item 1</div>
+    </sp-accordion-item>
+    <sp-accordion-item disabled label="Heading 2">
+        <div>Item 2</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 3">
+        <div>Item 3</div>
+    </sp-accordion-item>
+    <sp-accordion-item label="Heading 4">
+        <div>Item 4</div>
+    </sp-accordion-item>
+</sp-accordion>
+```
+
+</sp-tab-panel>
+</sp-tabs>
diff --git a/packages/accordion/src/Accordion.ts b/packages/accordion/src/Accordion.ts
@@ -49,7 +49,7 @@ export class Accordion extends SizedMixin(SpectrumElement, {
      * Sets the spacing between the content to borders of an accordion item
      */
     @property({ type: String, reflect: true })
-    public density: 'compact' | 'spacious' | undefined;
+    public density?: 'compact' | 'spacious';
 
     @queryAssignedNodes()
     private defaultNodes!: NodeListOf<AccordionItem>;
@@ -93,7 +93,7 @@ export class Accordion extends SizedMixin(SpectrumElement, {
         });
     }
 
-    private handleSlotchange({}: Event & { target: HTMLSlotElement }): void {
+    private handleSlotchange(): void {
         this.focusGroupController.clearElementCache();
         this.items.forEach((item) => {
             item.size = this.size;
@@ -102,7 +102,10 @@ export class Accordion extends SizedMixin(SpectrumElement, {
 
     protected override updated(changed: PropertyValues<this>): void {
         super.updated(changed);
-        if (changed.has('size')) {
+        if (
+            changed.has('size') &&
+            (!!changed.get('size') || this.size !== 'm')
+        ) {
             this.items.forEach((item) => {
                 item.size = this.size;
             });

diff --git a/packages/accordion/src/AccordionItem.ts b/packages/accordion/src/AccordionItem.ts
@@ -108,7 +108,7 @@ export class AccordionItem extends SizedMixin(Focusable, {
     }
 
     protected renderChevronIcon = (): TemplateResult => {
-        return chevronIcon[this.size]();
+        return chevronIcon[this.size || 'm']();
     };
 
     protected override render(): TemplateResult {

diff --git a/...on/stories/accordion-densities.stories.ts → ...es/accordion-densities-compact.stories.ts b/...on/stories/accordion-densities.stories.ts → ...es/accordion-densities-compact.stories.ts
@@ -14,11 +14,15 @@ import { TemplateResult } from '@spectrum-web-components/base';
 import { AccordionMarkup } from './';
 
 export default {
-    title: 'Accordion/Densities',
+    title: 'Accordion/Densities/Compact',
     component: 'sp-accordion',
 };
 
-export const compact = (): TemplateResult =>
-    AccordionMarkup({ density: 'compact' });
-export const spacious = (): TemplateResult =>
-    AccordionMarkup({ density: 'spacious' });
+export const s = (): TemplateResult =>
+    AccordionMarkup({ density: 'compact', size: 's' });
+export const m = (): TemplateResult =>
+    AccordionMarkup({ density: 'compact', size: 'm' });
+export const l = (): TemplateResult =>
+    AccordionMarkup({ density: 'compact', size: 'l' });
+export const xl = (): TemplateResult =>
+    AccordionMarkup({ density: 'compact', size: 'xl' });
diff --git a/packages/accordion/stories/accordion-densities-spacious.stories.ts b/packages/accordion/stories/accordion-densities-spacious.stories.ts
@@ -0,0 +1,28 @@
+/*
+Copyright 2023 Adobe. All rights reserved.
+This file is licensed to you 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 REPRESENTATIONS
+OF ANY KIND, either express or implied. See the License for the specific language
+governing permissions and limitations under the License.
+*/
+
+import { TemplateResult } from '@spectrum-web-components/base';
+import { AccordionMarkup } from './';
+
+export default {
+    title: 'Accordion/Densities/Spacious',
+    component: 'sp-accordion',
+};
+
+export const s = (): TemplateResult =>
+    AccordionMarkup({ density: 'spacious', size: 's' });
+export const m = (): TemplateResult =>
+    AccordionMarkup({ density: 'spacious', size: 'm' });
+export const l = (): TemplateResult =>
+    AccordionMarkup({ density: 'spacious', size: 'l' });
+export const xl = (): TemplateResult =>
+    AccordionMarkup({ density: 'spacious', size: 'xl' });