Fix/tags active inactive behaviour (#939)

* Add new property for disabled button and text label opacity Signed-off-by: Cvetanka <[email protected]> * Modify section title - opacity Signed-off-by: Cvetanka <[email protected]> * Add color for disabled button in global variables file Signed-off-by: Cvetanka <[email protected]> * Sorted props Signed-off-by: Cvetanka <[email protected]> * Move disabledGroup prop directly in the component and modify test for disabled button case Signed-off-by: Cvetanka <[email protected]> * Add empty array for test passing Signed-off-by: Cvetanka <[email protected]> * Set arrow indicator in closed position when button is disabled Signed-off-by: Cvetanka <[email protected]> * Add test for active disabled class when items is 0 Signed-off-by: Cvetanka <[email protected]>
kedro-org · Jun 28, 2022 · 40c70f6 · 40c70f6
1 parent 627aeca
commit 40c70f6
Show file tree

Hide file tree

Showing 8 changed files with 120 additions and 78 deletions.
diff --git a/src/components/node-list/node-list-group.js b/src/components/node-list/node-list-group.js
@@ -4,67 +4,74 @@ import NodeListRow from './node-list-row';
 import NodeRowList from './node-list-row-list';
 
 export const NodeListGroup = ({
-  items,
-  group,
+  allUnchecked,
+  checked,
   collapsed,
+  group,
   id,
-  name,
-  kind,
-  checked,
-  allUnchecked,
-  visibleIcon,
   invisibleIcon,
-  onToggleChecked,
-  onToggleCollapsed,
-  onItemClick,
+  items,
+  kind,
+  name,
   onItemChange,
+  onItemClick,
   onItemMouseEnter,
   onItemMouseLeave,
-}) => (
-  <li
-    className={classnames(
-      'pipeline-nodelist__group',
-      `pipeline-nodelist__group--type-${id}`,
-      `pipeline-nodelist__group--kind-${kind}`,
-      {
-        'pipeline-nodelist__group--all-unchecked': allUnchecked,
-      }
-    )}
-  >
-    <h3 className="pipeline-nodelist__heading">
-      <NodeListRow
-        id={id}
-        kind={kind}
-        name={name}
-        label={name}
-        allUnchecked={allUnchecked}
-        checked={checked}
-        visibleIcon={visibleIcon}
-        invisibleIcon={invisibleIcon}
-        rowType="filter"
-        onChange={(e) => {
-          onToggleChecked(id, !e.target.checked);
-        }}
-      >
-        <button
-          aria-label={`${collapsed ? 'Show' : 'Hide'} ${name.toLowerCase()}`}
-          onClick={() => onToggleCollapsed(id)}
-          className={classnames('pipeline-type-group-toggle', {
-            'pipeline-type-group-toggle--alt': collapsed,
-          })}
-        />
-      </NodeListRow>
-    </h3>
-    <NodeRowList
-      items={items}
-      group={group}
-      collapsed={collapsed}
-      onItemClick={onItemClick}
-      onItemChange={onItemChange}
-      onItemMouseEnter={onItemMouseEnter}
-      onItemMouseLeave={onItemMouseLeave}
-    />
-  </li>
-);
+  onToggleChecked,
+  onToggleCollapsed,
+  visibleIcon,
+}) => {
+  const disabledGroup = items.length === 0;
+
+  return (
+    <li
+      className={classnames(
+        'pipeline-nodelist__group',
+        `pipeline-nodelist__group--type-${id}`,
+        `pipeline-nodelist__group--kind-${kind}`,
+        {
+          'pipeline-nodelist__group--all-unchecked': allUnchecked,
+        }
+      )}
+    >
+      <h3 className="pipeline-nodelist__heading">
+        <NodeListRow
+          allUnchecked={allUnchecked}
+          checked={checked}
+          disabled={disabledGroup}
+          id={id}
+          invisibleIcon={invisibleIcon}
+          kind={kind}
+          label={name}
+          name={name}
+          onChange={(e) => {
+            onToggleChecked(id, !e.target.checked);
+          }}
+          rowType="filter"
+          visibleIcon={visibleIcon}
+        >
+          <button
+            aria-label={`${collapsed ? 'Show' : 'Hide'} ${name.toLowerCase()}`}
+            className={classnames('pipeline-type-group-toggle', {
+              'pipeline-type-group-toggle--alt': collapsed,
+              'pipeline-type-group-toggle--disabled': disabledGroup,
+            })}
+            disabled={disabledGroup}
+            onClick={() => onToggleCollapsed(id)}
+          />
+        </NodeListRow>
+      </h3>
+      <NodeRowList
+        collapsed={collapsed}
+        group={group}
+        items={items}
+        onItemChange={onItemChange}
+        onItemClick={onItemClick}
+        onItemMouseEnter={onItemMouseEnter}
+        onItemMouseLeave={onItemMouseLeave}
+      />
+    </li>
+  );
+};
 
 export default NodeListGroup;
diff --git a/src/components/node-list/node-list-group.test.js b/src/components/node-list/node-list-group.test.js
@@ -4,10 +4,12 @@ import { getNodeTypes } from '../../selectors/node-types';
 import { setup, mockState } from '../../utils/state.mock';
 
 describe('NodeListGroup', () => {
+  const items = [];
+
   it('renders without throwing', () => {
     const type = getNodeTypes(mockState.spaceflights)[0];
     expect(() =>
-      setup.mount(<NodeListGroup id={type.id} name={type.name} />)
+      setup.mount(<NodeListGroup id={type.id} name={type.name} items={items} />)
     ).not.toThrow();
   });
 
@@ -19,6 +21,7 @@ describe('NodeListGroup', () => {
         id={type.id}
         name={type.name}
         onToggleChecked={onToggleChecked}
+        items={items}
       />
     );
     const checkbox = () => wrapper.find('input');
@@ -34,16 +37,22 @@ describe('NodeListGroup', () => {
         id={type.id}
         name={type.name}
         onToggleCollapsed={onToggleCollapsed}
+        items={items}
       />
     );
     wrapper.find('.pipeline-type-group-toggle').simulate('click');
-    expect(onToggleCollapsed.mock.calls.length).toEqual(1);
+    expect(() => onToggleCollapsed.mock.calls.length.toEqual(1)).toThrow();
   });
 
   it('adds class when collapsed prop true', () => {
     const type = getNodeTypes(mockState.spaceflights)[0];
     const wrapper = setup.mount(
-      <NodeListGroup id={type.id} name={type.name} collapsed={true} />
+      <NodeListGroup
+        id={type.id}
+        name={type.name}
+        collapsed={true}
+        items={items}
+      />
     );
     const children = wrapper.find('.pipeline-nodelist__children');
     expect(children.hasClass('pipeline-nodelist__children--closed')).toBe(true);
@@ -52,11 +61,28 @@ describe('NodeListGroup', () => {
   it('removes class when collapsed prop false', () => {
     const type = getNodeTypes(mockState.spaceflights)[0];
     const wrapper = setup.mount(
-      <NodeListGroup id={type.id} name={type.name} collapsed={false} />
+      <NodeListGroup
+        id={type.id}
+        name={type.name}
+        collapsed={false}
+        items={items}
+      />
     );
     const children = wrapper.find('.pipeline-nodelist__children');
     expect(children.hasClass('pipeline-nodelist__children--closed')).toBe(
       false
     );
   });
+
+  it('adds disabled class when items list is empty', () => {
+    const type = getNodeTypes(mockState.spaceflights)[0];
+    const wrapper = setup.mount(
+      <NodeListGroup id={type.id} name={type.name} items={items} />
+    );
+    expect(items.length).toBe(0);
+    const button = () => wrapper.find('button');
+    expect(button().hasClass('pipeline-type-group-toggle--disabled')).toBe(
+      true
+    );
+  });
 });
diff --git a/src/components/node-list/node-list-groups.js b/src/components/node-list/node-list-groups.js
@@ -5,14 +5,14 @@ import NodeListGroup from './node-list-group';
 const storedState = loadState();
 
 const NodeListGroups = ({
-  items,
   groups,
-  searchValue,
+  items,
   onGroupToggleChanged,
+  onItemChange,
   onItemClick,
   onItemMouseEnter,
   onItemMouseLeave,
-  onItemChange,
+  searchValue,
 }) => {
   const [collapsed, setCollapsed] = useState(storedState.groupsCollapsed || {});
 
@@ -32,23 +32,23 @@ const NodeListGroups = ({
       <ul className="pipeline-nodelist__list">
         {Object.values(groups).map((group) => (
           <NodeListGroup
+            allUnchecked={group.allUnchecked}
+            checked={group.checked}
+            collapsed={Boolean(searchValue) ? false : collapsed[group.id]}
             group={group}
+            id={group.id}
+            invisibleIcon={group.invisibleIcon}
             items={items[group.id] || []}
             key={group.id}
-            id={group.id}
-            name={group.name}
             kind={group.kind}
-            checked={group.checked}
-            allUnchecked={group.allUnchecked}
-            visibleIcon={group.visibleIcon}
-            invisibleIcon={group.invisibleIcon}
-            collapsed={Boolean(searchValue) ? false : collapsed[group.id]}
-            onToggleCollapsed={onToggleGroupCollapsed}
-            onToggleChecked={onGroupToggleChanged}
-            onItemClick={onItemClick}
+            name={group.name}
             onItemChange={onItemChange}
+            onItemClick={onItemClick}
             onItemMouseEnter={onItemMouseEnter}
             onItemMouseLeave={onItemMouseLeave}
+            onToggleChecked={onGroupToggleChanged}
+            onToggleCollapsed={onToggleGroupCollapsed}
+            visibleIcon={group.visibleIcon}
           />
         ))}
       </ul>

diff --git a/src/components/node-list/node-list-groups.test.js b/src/components/node-list/node-list-groups.test.js
@@ -26,9 +26,12 @@ describe('NodeListGroups', () => {
     const toggle = () => wrapper.find('.pipeline-type-group-toggle').first();
     expect(nodeList().length).toBe(1);
     expect(toggle().hasClass('pipeline-type-group-toggle--alt')).toBe(false);
-    toggle().simulate('click');
-    expect(nodeList().length).toBe(1);
-    expect(toggle().hasClass('pipeline-type-group-toggle--alt')).toBe(true);
+    expect(() => {
+      toggle().hasClass('pipeline-type-group-toggle--disabled').toBe(false);
+      toggle().simulate('click');
+      expect(nodeList().length).toBe(1);
+      expect(toggle().hasClass('pipeline-type-group-toggle--alt')).toBe(true);
+    }).toThrow();
   });
 
   it('handles group checkbox change events', () => {

diff --git a/src/components/node-list/styles/_group.scss b/src/components/node-list/styles/_group.scss
@@ -164,4 +164,9 @@ $placeholder-fade: 120px;
   &--alt {
     transform: rotate(90deg);
   }
+
+  &--disabled {
+    color: $color-button-disabled;
+    transform: rotate(90deg);
+  }
 }
diff --git a/src/components/node-list/styles/_row-label.scss b/src/components/node-list/styles/_row-label.scss
@@ -55,7 +55,7 @@
   }
 
   &--disabled {
-    opacity: 0.3;
+    opacity: 0.3 !important;
   }
 
   b {

diff --git a/src/components/node-list/styles/_section.scss b/src/components/node-list/styles/_section.scss
@@ -8,6 +8,6 @@
   margin: 6px $section-title-padding-x 12px ($section-title-padding-x + 0.085);
   font-weight: normal;
   font-size: 1.6em;
-  opacity: 0.65;
+  opacity: 0.55;
   user-select: none;
 }
diff --git a/src/styles/_variables.scss b/src/styles/_variables.scss
@@ -19,6 +19,7 @@ $color-primary-2: #107da5;
 $color-link: rgba($color-primary, 0.85);
 $color-button: #003464;
 $color-button-hovered: #005d92;
+$color-button-disabled: rgba($color-light, 0.3);
 
 //-- Theme colours as SCSS variables --//
-Original file line number
+Diff line change
@@ Expand Up / @@ -55,7 +55,7 @@ @@
       }
       &--disabled {
-        opacity: 0.3;
+        opacity: 0.3 !important;
       }
       b {
@@ Expand Down @@