fix: remove margin on glyph when there is no label (#1757)

* put in a check to remove margin on glyph * add test for hasGlyphAndContent * create isNullOrUndefined function * fix data-sketch-symbol and capitalize A * fix data-sketch-symbol * removed isNullOrUndefined to use lodash-es isNil * revert action toggle and auto suggest option work * revert action toggle spec changes
microsoft · Jun 4, 2019 · 4179abf · 4179abf
1 parent 1325228
commit 4179abf
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 3 deletions.
diff --git a/packages/fast-components-class-name-contracts-msft/src/action-trigger/index.ts b/packages/fast-components-class-name-contracts-msft/src/action-trigger/index.ts
@@ -40,4 +40,9 @@ export interface ActionTriggerClassNameContract {
      * The disabled modifier
      */
     actionTrigger__disabled?: string;
+
+    /**
+     * Action trigger class name used when checking glyph and content
+     */
+    actionTrigger__hasGlyphAndContent?: string;
 }
diff --git a/packages/fast-components-react-msft/src/action-trigger/action-trigger.spec.tsx b/packages/fast-components-react-msft/src/action-trigger/action-trigger.spec.tsx
@@ -29,6 +29,7 @@ describe("action trigger", (): void => {
         actionTrigger__primary: "action-trigger-primary",
         actionTrigger__stealth: "action-trigger-stealth",
         actionTrigger__disabled: "action-trigger-disabled",
+        actionTrigger__hasGlyphAndContent: "action-trigger-has-glyph-and-content",
     };
     const href: string = "#";
 
@@ -212,4 +213,33 @@ describe("action trigger", (): void => {
 
         expect(rendered.find("button").prop("className")).toContain("custom-class-name");
     });
+
+    test("should apply 'hasGlyphandContent' class when display includes content and glyph", () => {
+        const props: ActionTriggerHandledProps = {
+            glyph: (className?: string): React.ReactNode => {
+                return <div>X</div>;
+            },
+            children: "text",
+        };
+
+        const rendered: any = mount(<ActionTrigger {...props} />);
+
+        expect(rendered.find("button").prop("className")).toContain(
+            "actionTrigger__hasGlyphAndContent"
+        );
+    });
+
+    test("should not apply 'hasGlyphandContent' class when displaying only glyph", () => {
+        const props: ActionTriggerHandledProps = {
+            glyph: (className?: string): React.ReactNode => {
+                return <div>X</div>;
+            },
+        };
+
+        const rendered: any = mount(<ActionTrigger {...props} />);
+
+        expect(rendered.find("button").prop("className")).not.toContain(
+            "actionTrigger__hasGlyphAndContent"
+        );
+    });
 });
diff --git a/packages/fast-components-react-msft/src/action-trigger/action-trigger.tsx b/packages/fast-components-react-msft/src/action-trigger/action-trigger.tsx
@@ -1,5 +1,5 @@
 import React from "react";
-import { get } from "lodash-es";
+import { get, isNil } from "lodash-es";
 import Foundation, { HandledProps } from "@microsoft/fast-components-foundation-react";
 import { Button, ButtonAppearance } from "../button";
 import {
@@ -68,12 +68,27 @@ class ActionTrigger extends Foundation<
             )}`;
         }
 
+        if (this.hasGlyphAndContent()) {
+            classNames = `${classNames} ${get(
+                this.props,
+                "managedClasses.actionTrigger__hasGlyphAndContent",
+                ""
+            )}`;
+        }
+
         return super.generateClassNames(classNames);
     }
 
     private generateGlyph = (): React.ReactNode => {
         return this.props.glyph(get(this.props, "managedClasses.actionTrigger_glyph"));
     };
+
+    /**
+     * Checks to see if action trigger is displaying both glyph and content or not
+     */
+    private hasGlyphAndContent(): boolean {
+        return !isNil(this.props.glyph) && !isNil(this.props.children);
+    }
 }
 
 export default ActionTrigger;

diff --git a/packages/fast-components-react-msft/src/action-trigger/examples.data.tsx b/packages/fast-components-react-msft/src/action-trigger/examples.data.tsx
@@ -81,6 +81,15 @@ const examples: ComponentFactoryExample<ActionTriggerProps> = {
             },
             "data-sketch-symbol": "Action trigger - stealth",
         } as any,
+        {
+            appearance: ActionTriggerAppearance.stealth,
+            href: testDestination,
+            glyph: {
+                id: svgSchema.id,
+                props: {},
+            },
+            "data-sketch-symbol": "Action trigger - stealth - only icon",
+        } as any,
     ],
 };
 

diff --git a/packages/fast-components-styles-msft/src/action-trigger/index.ts b/packages/fast-components-styles-msft/src/action-trigger/index.ts
@@ -38,8 +38,6 @@ const styles: ComponentStyles<ActionTriggerClassNameContract, DesignSystem> = {
         width: glyphSize,
         height: glyphSize,
         flexShrink: "0",
-        marginRight: directionSwitch(horizontalSpacing(), ""),
-        marginLeft: directionSwitch("", horizontalSpacing()),
     },
     actionTrigger__primary: {
         "& $actionTrigger_glyph": {
@@ -102,6 +100,12 @@ const styles: ComponentStyles<ActionTriggerClassNameContract, DesignSystem> = {
         },
     },
     actionTrigger__disabled: {},
+    actionTrigger__hasGlyphAndContent: {
+        "& $actionTrigger_glyph": {
+            marginRight: directionSwitch(horizontalSpacing(), ""),
+            marginLeft: directionSwitch("", horizontalSpacing()),
+        },
+    },
 };
 
 export default styles;