feat: change the way we set default values on button

dnbexperience · Nov 26, 2018 · 01a986a · 01a986a
1 parent 62e5dd0
commit 01a986a
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 82 deletions.
diff --git a/packages/dnb-ui-lib/src/components/button/Button.js b/packages/dnb-ui-lib/src/components/button/Button.js
@@ -58,8 +58,8 @@ export const propTypes = {
 export const defaultProps = {
   type: 'button',
   text: null,
-  variant: 'primary',
-  size: 'default',
+  variant: null,
+  size: null,
   title: null,
   icon: null,
   icon_position: 'right',
@@ -145,8 +145,19 @@ export default class Button extends PureComponent {
 
     // if only has Icon, then resize it and define it as secondary
     if (!text && icon) {
-      variant = 'secondary'
-      size = 'medium'
+      if (!variant) {
+        variant = 'secondary'
+      }
+      if (!size) {
+        size = 'medium'
+      }
+    } else if (text) {
+      if (!variant) {
+        variant = 'primary'
+      }
+      if (!size) {
+        size = 'default'
+      }
     }
 
     const content = Button.getContent(this.props)
@@ -242,18 +253,14 @@ class Content extends PureComponent {
     if (icon) {
       const alt = title || text
       ret.push(
-        <span
+        <Icon
           key="button-icon"
           className="dnb-button__icon"
-          role="presentation"
-        >
-          <Icon
-            icon={icon}
-            size={icon_size}
-            alt={alt}
-            area_hidden={Boolean(alt)}
-          />
-        </span>
+          icon={icon}
+          size={icon_size}
+          alt={alt}
+          area_hidden={Boolean(alt)}
+        />
       )
     }
 

diff --git a/packages/dnb-ui-lib/src/components/button/style/themes/dnb-button-theme-ui.scss b/packages/dnb-ui-lib/src/components/button/style/themes/dnb-button-theme-ui.scss
@@ -69,7 +69,7 @@
     }
   }
 
-  &:not([disabled]):hover {
+  &.dnb-button--has-text:not([disabled]):hover {
     @include drawUnderlineLine() {
       left: 0.45em;
       right: 0.45em;
@@ -82,6 +82,14 @@
       right: 0.7em;
     }
   }
+
+  // icon only tertiary
+  &.dnb-button--has-icon:not(.dnb-button--has-text):hover {
+    .dnb-button__icon {
+      color: var(--color-sea-green-alt-30);
+    }
+  }
+
   &:not([disabled]):active::after,
   html[data-whatinput='keyboard'] &:not([disabled]):focus::after {
     border-bottom: none;

diff --git a/packages/dnb-ui-lib/src/components/icon/Icon.js b/packages/dnb-ui-lib/src/components/icon/Icon.js
@@ -170,11 +170,12 @@ export default class Icon extends PureComponent {
     // some wrapper params
     // also used for code markup simulation
     const wrapperParams = validateDOMAttributes(props, {
-      role: 'presentation'
+      role: 'img'
     })
     // get the alt
     wrapperParams['aria-label'] = (alt || name).replace(/_/g, ' ')
     if (area_hidden) {
+      // wrapperParams['role'] = 'presentation' // almost the same as aria-hidden
       wrapperParams['aria-hidden'] = area_hidden
     }
     wrapperParams.className = classnames(

diff --git a/packages/dnb-ui-lib/src/components/icon/__tests__/__snapshots__/Icon.test.js.snap b/packages/dnb-ui-lib/src/components/icon/__tests__/__snapshots__/Icon.test.js.snap
@@ -18,7 +18,7 @@ exports[`Icon component have to match snapshot 1`] = `
     aria-hidden={true}
     aria-label="alt"
     className="dnb-icon dnb-icon--modifier has-custom-size class className"
-    role="presentation"
+    role="img"
   >
     <SvgComponent
       color="color"

diff --git a/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap b/packages/dnb-ui-lib/src/components/modal/__tests__/__snapshots__/Modal.test.js.snap
@@ -36,7 +36,7 @@ exports[`Modal component have to match snapshot 1`] = `
         id={null}
         innerRef={null}
         on_click={[Function]}
-        size="default"
+        size={null}
         text="modal_trigger_text"
         title="modal_trigger_title"
         type="button"
@@ -65,7 +65,7 @@ exports[`Modal component have to match snapshot 1`] = `
             id={null}
             innerRef={null}
             on_click={[Function]}
-            size="default"
+            size={null}
             text="modal_trigger_text"
             title="modal_trigger_title"
             type="button"
@@ -77,25 +77,20 @@ exports[`Modal component have to match snapshot 1`] = `
             >
               modal_trigger_text
             </span>
-            <span
+            <IconWithAllIcons
+              alt="modal_trigger_title"
+              area_hidden={true}
+              attributes={null}
+              class={null}
               className="dnb-button__icon"
+              color={null}
+              height={null}
+              icon="modal_trigger_icon"
               key="button-icon"
-              role="presentation"
-            >
-              <IconWithAllIcons
-                alt="modal_trigger_title"
-                area_hidden={true}
-                attributes={null}
-                class={null}
-                className={null}
-                color={null}
-                height={null}
-                icon="modal_trigger_icon"
-                modifier={null}
-                size={16}
-                width={null}
-              />
-            </span>
+              modifier={null}
+              size={16}
+              width={null}
+            />
           </Content>
         </button>
       </Button>
@@ -168,7 +163,7 @@ exports[`Modal component have to match snapshot 1`] = `
                 id={null}
                 innerRef={null}
                 on_click={[Function]}
-                size="default"
+                size={null}
                 text={null}
                 title="close_title"
                 type="button"
@@ -197,55 +192,50 @@ exports[`Modal component have to match snapshot 1`] = `
                     id={null}
                     innerRef={null}
                     on_click={[Function]}
-                    size="default"
+                    size={null}
                     text={null}
                     title="close_title"
                     type="button"
                     variant="secondary"
                   >
-                    <span
+                    <IconWithAllIcons
+                      alt="close_title"
+                      area_hidden={true}
+                      attributes={null}
+                      class={null}
                       className="dnb-button__icon"
+                      color={null}
+                      height={null}
+                      icon="close"
                       key="button-icon"
-                      role="presentation"
+                      modifier={null}
+                      size={16}
+                      width={null}
                     >
-                      <IconWithAllIcons
-                        alt="close_title"
-                        area_hidden={true}
-                        attributes={null}
-                        class={null}
-                        className={null}
-                        color={null}
-                        height={null}
-                        icon="close"
-                        modifier={null}
-                        size={16}
-                        width={null}
+                      <span
+                        aria-hidden={true}
+                        aria-label="close title"
+                        className="dnb-icon dnb-icon--default dnb-button__icon"
+                        role="img"
                       >
-                        <span
-                          aria-hidden={true}
-                          aria-label="close title"
-                          className="dnb-icon dnb-icon--default"
-                          role="presentation"
+                        <close
+                          height={null}
+                          width={null}
                         >
-                          <close
+                          <svg
                             height={null}
+                            viewBox="0 0 16 16"
                             width={null}
                           >
-                            <svg
-                              height={null}
-                              viewBox="0 0 16 16"
-                              width={null}
-                            >
-                              <path
-                                clipRule="evenodd"
-                                d="M4.03 3.1a.75.75 0 0 0-1.06 1.06l3.97 3.97-3.97 3.97a.75.75 0 1 0 1.06 1.06L8 9.19l3.97 3.97a.75.75 0 1 0 1.06-1.06L9.06 8.13l3.97-3.97a.75.75 0 0 0-1.06-1.06L8 7.07 4.03 3.1z"
-                                fillRule="evenodd"
-                              />
-                            </svg>
-                          </close>
-                        </span>
-                      </IconWithAllIcons>
-                    </span>
+                            <path
+                              clipRule="evenodd"
+                              d="M4.03 3.1a.75.75 0 0 0-1.06 1.06l3.97 3.97-3.97 3.97a.75.75 0 1 0 1.06 1.06L8 9.19l3.97 3.97a.75.75 0 1 0 1.06-1.06L9.06 8.13l3.97-3.97a.75.75 0 0 0-1.06-1.06L8 7.07 4.03 3.1z"
+                              fillRule="evenodd"
+                            />
+                          </svg>
+                        </close>
+                      </span>
+                    </IconWithAllIcons>
                   </Content>
                 </button>
               </Button>

diff --git a/packages/dnb-ui-lib/src/shared/component-helper.js b/packages/dnb-ui-lib/src/shared/component-helper.js
@@ -99,21 +99,22 @@ export const processChildren = props => {
     typeof props.children === 'function'
       ? props.children(props)
       : props.children
+
+  // if we get several react children witch representates only a text
   if (Array.isArray(res)) {
-    return res.reduce((pV, cV) => {
+    const onlyTexts = res.reduce((pV, cV) => {
       if (typeof cV === 'string') {
-        pV += cV
-      }
-      if (typeof cV === 'object') {
-        if (typeof pV === 'string') {
-          pV = []
-        }
         pV.push(cV)
-        return pV
       }
       return pV
-    }, '')
+    }, [])
+
+    // if there was more than one text elements
+    if (onlyTexts.length > 1) {
+      return onlyTexts.join('')
+    }
   }
+
   return res
 }