fix(button): update logic and tests to be more clear and pass all tests

Kajabi · Dec 12, 2022 · 2014e7b · 2014e7b
1 parent 9fee1c7
commit 2014e7b
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 31 deletions.
diff --git a/libs/core/src/components.d.ts b/libs/core/src/components.d.ts
@@ -46,7 +46,7 @@ export namespace Components {
         /**
           * Sets button variant styles as outlined in Figma documentation
          */
-        "variant"?: 'secondary' | 'accent' | 'disclosure' | 'destructive';
+        "variant": 'primary' | 'secondary' | 'accent' | 'disclosure' | 'destructive';
     }
     interface SageLink {
         /**
@@ -135,7 +135,7 @@ declare namespace LocalJSX {
         /**
           * Sets button variant styles as outlined in Figma documentation
          */
-        "variant"?: 'secondary' | 'accent' | 'disclosure' | 'destructive';
+        "variant"?: 'primary' | 'secondary' | 'accent' | 'disclosure' | 'destructive';
     }
     interface SageLink {
         /**

diff --git a/libs/core/src/components/sage-button/readme.md b/libs/core/src/components/sage-button/readme.md
@@ -5,14 +5,14 @@
 
 ## Properties
 
-| Property   | Attribute  | Description                                                             | Type                                                       | Default     |
-| ---------- | ---------- | ----------------------------------------------------------------------- | ---------------------------------------------------------- | ----------- |
-| `disabled` | `disabled` | Toggles disabled state of button                                        | `boolean`                                                  | `false`     |
-| `icon`     | `icon`     | Displays icon before text when icon string matches an icon name         | `string`                                                   | `null`      |
-| `name`     | `name`     | Provides button with a submittable name                                 | `string`                                                   | `undefined` |
-| `type`     | `type`     | Provides button with a type  todo: make sure there is info in the story | `"button" \| "reset" \| "submit"`                          | `'button'`  |
-| `value`    | `value`    | Provides button with a submittable value                                | `string`                                                   | `undefined` |
-| `variant`  | `variant`  | Sets button variant styles as outlined in Figma documentation           | `"accent" \| "destructive" \| "disclosure" \| "secondary"` | `undefined` |
+| Property   | Attribute  | Description                                                             | Type                                                                    | Default     |
+| ---------- | ---------- | ----------------------------------------------------------------------- | ----------------------------------------------------------------------- | ----------- |
+| `disabled` | `disabled` | Toggles disabled state of button                                        | `boolean`                                                               | `false`     |
+| `icon`     | `icon`     | Displays icon before text when icon string matches an icon name         | `string`                                                                | `null`      |
+| `name`     | `name`     | Provides button with a submittable name                                 | `string`                                                                | `undefined` |
+| `type`     | `type`     | Provides button with a type  todo: make sure there is info in the story | `"button" \| "reset" \| "submit"`                                       | `'button'`  |
+| `value`    | `value`    | Provides button with a submittable value                                | `string`                                                                | `undefined` |
+| `variant`  | `variant`  | Sets button variant styles as outlined in Figma documentation           | `"accent" \| "destructive" \| "disclosure" \| "primary" \| "secondary"` | `'primary'` |
 
 
 ----------------------------------------------

diff --git a/libs/core/src/components/sage-button/sage-button.tsx b/libs/core/src/components/sage-button/sage-button.tsx
@@ -12,7 +12,7 @@ export class SageButton {
   /**
    * Sets button variant styles as outlined in Figma documentation
     */
-  @Prop() variant?: 'secondary' | 'accent' | 'disclosure' | 'destructive';
+  @Prop() variant: 'primary' | 'secondary' | 'accent' | 'disclosure' | 'destructive' = 'primary';
 
   /**
    * Displays icon before text when icon string matches an icon name
@@ -45,7 +45,7 @@ export class SageButton {
 
   // todo build out e2e test to confirm form submission 
   private handleClick = (ev: Event) => {
-    if (this.type === 'button') {
+    if (this.type != 'button') {
       // If button clicked IS NOT associated with a form
       if (hasShadowDom(this.el)) {
         const form = this.el.closest('form')
@@ -64,9 +64,19 @@ export class SageButton {
       // If button clicked IS associated with a form
     }
   }
-
-  private buttonClassNames = `sage-button` + (this.variant && ` sage-button--${this.variant}`);
 
+  private buttonClassNames = () => {
+    let className = `sage-button`;
+    // console.log(className);
+    if (this.variant && this.variant != 'primary') {
+      const variantClassName = `sage-button--${this.variant}`;
+      className += ' ' + variantClassName;
+    }
+    // return className;
+    return className;
+    // console.log(className);
+  };
+
   render() {
     const trashIcon = (
       <svg width="16" height="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg">
@@ -78,7 +88,7 @@ export class SageButton {
 
     return (
       <Host variant={this.variant} aria-disabled={this.disabled ? 'true' : null} onClick={this.handleClick}>
-        <button class={this.buttonClassNames} disabled={this.disabled} type={this.type} name={this.name} value={this.value} >
+        <button class={this.buttonClassNames()} disabled={this.disabled} type={this.type} name={this.name} value={this.value} >
           {this.icon == 'trashIcon' && trashIcon}
           <slot />
         </button>

diff --git a/libs/core/src/components/sage-button/stories/sage-button.stories.mdx b/libs/core/src/components/sage-button/stories/sage-button.stories.mdx
@@ -39,6 +39,8 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
 
 ### Default (Primary)
 
+Default button styles, used with affirmative actions such as submitting, continuing, and saving
+
 <Canvas>
   <Story name="Primary" args={{ icon: '', disabled: false, slot: 'Default', type: 'button' }}>
     {Template.bind()}
@@ -53,7 +55,7 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
 
 ### Secondary
 
-// Todo define variants
+Secondary button styles, used with neutral actions such as going back, saving as draft, and cancelling
 
 <Canvas>
   <Story name="Secondary" args={{ variant: 'secondary', icon: '', disabled: false, slot: 'Default', type: 'button' }}>
@@ -69,6 +71,8 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
 
 ### Accent
 
+Accent button styles, used for tertiary actions or when more contrast is needed
+
 <Canvas>
   <Story name="Accent" args={{ variant: 'accent', icon: '', disabled: false, slot: 'Default', type: 'button' }}>
     {Template.bind()}
@@ -83,6 +87,8 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
 
 ### Disclosure
 
+Disclosure button styles, used to provide multiple options or more information within a button
+
 <Canvas>
   <Story name="Disclosure" args={{ variant: 'disclosure', icon: '', disabled: false, slot: 'Default', type: 'button' }}>
     {Template.bind()}
@@ -97,6 +103,8 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
 
 ### Destructive
 
+Destructive button styles, used with actions that delete information such as deleting an item or removing a contact
+
 <Canvas>
   <Story name="Destructive" args={{ variant: 'destructive', icon: '', disabled: false, slot: 'Default', type: 'button' }}>
     {Template.bind()}
@@ -109,6 +117,24 @@ A button component to generate all versions of `sage-button` as outlined in [Fig
   </Story>
 </Canvas>
 
+## Type
+
+The type prop allows a `sage-button` to set [button type attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attr-type).
+
+### Default (Button)
+
+The button has no default behavior, and does nothing when pressed by default. It can have client-side scripts listen to the element's events, which are triggered when the events occur.
+
+This is our default button type.
+
+### Submit
+
+The button submits the form data to the server.
+
+### Reset
+
+The button resets all the controls to their initial values. (This behavior tends to annoy users.)
+
 ## Additional Resources
 
 [CSS Tricks - Buttons vs. Links](https://css-tricks.com/buttons-vs-links/)
diff --git a/libs/core/src/components/sage-button/test/sage-button.e2e.ts b/libs/core/src/components/sage-button/test/sage-button.e2e.ts
@@ -8,6 +8,49 @@ describe('sage-button', () => {
     expect(element).toHaveClass('hydrated');
   });
 
+  it('submits form', async () => {
+    const page = await newE2EPage({
+      html: `
+        <form>
+          <sage-button type="submit"></sage-button>
+        </form>
+      `
+    });
+
+    const elementForm = await page.find('form');
+    const elementFormEvent = await elementForm.spyOnEvent('submit');
+    await page.evaluate(() => document.querySelector('sage-button').click());
+    await page.waitForChanges();
+
+    // Confirm onClick has been called
+    expect(elementFormEvent).toHaveReceivedEvent();
+  });
+
+
+  it('resets form', async () => {
+    const page = await newE2EPage({
+      html: `
+        <form>
+          <input></input>
+          <sage-button type="reset"></sage-button>
+        </form>
+      `
+    });
+
+    const elementForm = await page.find('form');
+    const elementFormEvent = await elementForm.spyOnEvent('reset');
+    page.evaluate(() => document.querySelector('input').value = 'test');
+    await page.evaluate(() => document.querySelector('sage-button').click());    
+    await page.waitForChanges();
+
+    // Confirm form received reset event
+    expect(elementFormEvent).toHaveReceivedEvent();
+
+    // Confirm input value was reset
+    const updatedFormInputValue = await page.evaluate(() => document.querySelector('input').value);
+    expect(updatedFormInputValue).toBe('');
+  });
+
   it('renders when slot is used', async () => {
     const page = await newE2EPage();
     await page.setContent('<sage-button>Test Content</sage-button>');

diff --git a/libs/core/src/components/sage-button/test/sage-button.spec.tsx b/libs/core/src/components/sage-button/test/sage-button.spec.tsx
@@ -8,9 +8,9 @@ describe('sage-button', () => {
       html: `<sage-button></sage-button>`,
     });
     expect(root).toEqualHtml(`
-      <sage-button>
+      <sage-button variant="primary">
         <mock:shadow-root>
-          <button class="sage-button sage-button--primary">
+          <button class="sage-button" type="button">
             <slot></slot>
           </button>
         </mock:shadow-root>
@@ -26,7 +26,7 @@ describe('sage-button', () => {
     expect(root).toEqualHtml(`
       <sage-button variant="accent">
         <mock:shadow-root>
-          <button class="sage-button sage-button--accent">
+          <button class="sage-button sage-button--accent" type="button">
             <slot></slot>
           </button>
         </mock:shadow-root>
@@ -40,9 +40,9 @@ describe('sage-button', () => {
       html: `<sage-button disabled="true"></sage-button>`,
     });
     expect(root).toEqualHtml(`
-      <sage-button disabled="true" aria-disabled="true">
+      <sage-button disabled="true" aria-disabled="true" variant="primary">
         <mock:shadow-root>
-          <button class="sage-button sage-button--primary" disabled>
+          <button class="sage-button" type="button" disabled>
             <slot></slot>
           </button>
         </mock:shadow-root>

diff --git a/package.json b/package.json
@@ -32,5 +32,6 @@
     "commitizen": {
       "path": "./node_modules/cz-conventional-changelog"
     }
-  }
+  },
+  "dependencies": {}
 }
diff --git a/yarn.lock b/yarn.lock
@@ -5891,15 +5891,10 @@ camelcase@^6.2.0:
   resolved "https://registry.npmjs.org/camelcase/-/camelcase-6.3.0.tgz"
   integrity sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA==
 
-caniuse-lite@^1.0.30001109:
-  version "1.0.30001420"
-  resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001420.tgz#f62f35f051e0b6d25532cf376776d41e45b47ef6"
-  integrity sha512-OnyeJ9ascFA9roEj72ok2Ikp7PHJTKubtEJIQ/VK3fdsS50q4KWy+Z5X0A1/GswEItKX0ctAp8n4SYDE7wTu6A==
-
-caniuse-lite@^1.0.30001400:
-  version "1.0.30001418"
-  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001418.tgz"
-  integrity sha512-oIs7+JL3K9JRQ3jPZjlH6qyYDp+nBTCais7hjh0s+fuBwufc7uZ7hPYMXrDOJhV360KGMTcczMRObk0/iMqZRg==
+caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001400:
+  version "1.0.30001439"
+  resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001439.tgz"
+  integrity sha512-1MgUzEkoMO6gKfXflStpYgZDlFM7M/ck/bgfVCACO5vnAf0fXoNVHdWtqGU+MYca+4bL9Z5bpOVmR33cWW9G2A==
 
 capture-exit@^2.0.0:
   version "2.0.0"