From 63431e426b3464c246a02dadcca09309942709e4 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 15 Oct 2024 16:16:43 +0200 Subject: [PATCH 01/46] update classnames to match the component name Signed-off-by: Huong Nguyen --- src/components/node-list/node-list-row.js | 54 +++++++++---------- .../node-list/styles/_row-label.scss | 12 ++--- .../node-list/styles/_row-toggle.scss | 20 +++---- src/components/node-list/styles/_row.scss | 18 +++---- 4 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js index fdabf9d584..b145dec2d7 100644 --- a/src/components/node-list/node-list-row.js +++ b/src/components/node-list/node-list-row.js @@ -77,16 +77,16 @@ const NodeListRow = memo( return ( {typeof count === 'number' && ( - + {count} )} @@ -157,7 +157,7 @@ const NodeListRow = memo( > * { opacity: $element-icon-opacity-1; } @@ -90,7 +90,7 @@ $element-icon-opacity-2: 1; } } - .pipeline-nodelist__row &:hover { + .node-list-row &:hover { > * { opacity: $element-icon-opacity-2; } @@ -156,13 +156,13 @@ $filter-icon-opacity-3: 1; } } - .pipeline-nodelist__row:hover & { + .node-list-row:hover & { > * { opacity: $filter-icon-opacity-1; } } - .pipeline-nodelist__row:hover & { + .node-list-row:hover & { &.pipeline-row__toggle-icon--parent:hover { > * { opacity: $filter-icon-opacity-2; @@ -170,7 +170,7 @@ $filter-icon-opacity-3: 1; } } - .pipeline-nodelist__row & { + .node-list-row & { &.pipeline-row__toggle-icon--checked { > * { opacity: $filter-icon-opacity-2; @@ -178,7 +178,7 @@ $filter-icon-opacity-3: 1; } } - .pipeline-nodelist__row:hover & { + .node-list-row:hover & { &.pipeline-row__toggle-icon--child { &.pipeline-row__toggle-icon--checked { > * { @@ -188,7 +188,7 @@ $filter-icon-opacity-3: 1; } } - .pipeline-nodelist__row & { + .node-list-row & { &.pipeline-row__toggle-icon--parent:hover { &.pipeline-row__toggle-icon--checked { > * { @@ -224,7 +224,7 @@ $filter-icon-opacity-3: 1; stroke: var(--color-nodelist-filter-indicator-off); } - .pipeline-nodelist__row:hover &.pipeline-row__toggle-icon--all-unchecked, + .node-list-row:hover &.pipeline-row__toggle-icon--all-unchecked, &.pipeline-row__toggle-icon--parent { fill: colors.$blue-300; stroke: colors.$blue-300; diff --git a/src/components/node-list/styles/_row.scss b/src/components/node-list/styles/_row.scss index 409be89666..37bcaaffe5 100644 --- a/src/components/node-list/styles/_row.scss +++ b/src/components/node-list/styles/_row.scss @@ -5,7 +5,7 @@ z-index: var.$zindex-MuiTreeItem-icon; } -.pipeline-nodelist__row { +.node-list-row { position: relative; display: flex; align-items: center; @@ -64,19 +64,19 @@ } } -.pipeline-nodelist__row--active::before, -.pipeline-nodelist__row--selected::before, -.pipeline-nodelist__row:hover::before { +.node-list-row--active::before, +.node-list-row--selected::before, +.node-list-row:hover::before { opacity: 1; } -.pipeline-nodelist__row--overwrite::before { +.node-list-row--overwrite::before { .Mui-selected & { opacity: 1; } } -.pipeline-nodelist__row__icon { +.node-list-row__icon { display: block; flex-shrink: 0; width: variables.$row-icon-size; @@ -92,7 +92,7 @@ } } -.pipeline-nodelist__row__type-icon { +.node-list-row__type-icon { &--nested > * { opacity: 0.3; } @@ -103,8 +103,8 @@ &--active, &--selected, - .pipeline-nodelist__row--visible:hover &, - [data-whatintent='keyboard'] .pipeline-nodelist__row__text:focus & { + .node-list-row--visible:hover &, + [data-whatintent='keyboard'] .node-list-row__text:focus & { > * { opacity: 1; } From e1ab7401e0493c5d1bb4ec8bd935e53298ce83d1 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 15 Oct 2024 16:19:48 +0200 Subject: [PATCH 02/46] update names in tests Signed-off-by: Huong Nguyen --- .../node-list/node-list-row.test.js | 34 +++++++++--------- src/components/node-list/node-list.test.js | 36 +++++++++---------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/components/node-list/node-list-row.test.js b/src/components/node-list/node-list-row.test.js index f4651e200f..5414a6b20b 100644 --- a/src/components/node-list/node-list-row.test.js +++ b/src/components/node-list/node-list-row.test.js @@ -31,7 +31,7 @@ describe('NodeListRow', () => { it('handles mouseenter events', () => { const { props } = setupProps(); const wrapper = setup.mount(); - const nodeRow = () => wrapper.find('.pipeline-nodelist__row'); + const nodeRow = () => wrapper.find('.node-list-row'); nodeRow().simulate('mouseenter'); expect(props.onMouseEnter.mock.calls.length).toEqual(1); }); @@ -39,7 +39,7 @@ describe('NodeListRow', () => { it('handles mouseleave events', () => { const { props } = setupProps(); const wrapper = setup.mount(); - const nodeRow = () => wrapper.find('.pipeline-nodelist__row'); + const nodeRow = () => wrapper.find('.node-list-row'); nodeRow().simulate('mouseleave'); expect(props.onMouseLeave.mock.calls.length).toEqual(1); }); @@ -51,8 +51,8 @@ describe('NodeListRow', () => { ); expect( activeNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--overwrite') + .find('.node-list-row') + .hasClass('node-list-row--overwrite') ).toBe(true); }); @@ -63,8 +63,8 @@ describe('NodeListRow', () => { ); expect( activeNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--overwrite') + .find('.node-list-row') + .hasClass('node-list-row--overwrite') ).toBe(true); }); @@ -75,8 +75,8 @@ describe('NodeListRow', () => { ); expect( activeNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--overwrite') + .find('.node-list-row') + .hasClass('node-list-row--overwrite') ).toBe(false); }); @@ -87,8 +87,8 @@ describe('NodeListRow', () => { ); expect( activeNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--overwrite') + .find('.node-list-row') + .hasClass('node-list-row--overwrite') ).toBe(false); }); @@ -99,8 +99,8 @@ describe('NodeListRow', () => { ); expect( activeNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--active') + .find('.node-list-row') + .hasClass('node-list-row--active') ).toBe(true); }); @@ -111,8 +111,8 @@ describe('NodeListRow', () => { ); expect( disabledNodeWrapper - .find('.pipeline-nodelist__row') - .hasClass('pipeline-nodelist__row--disabled') + .find('.node-list-row') + .hasClass('node-list-row--disabled') ).toBe(true); }); @@ -120,7 +120,7 @@ describe('NodeListRow', () => { const { props } = setupProps(); const mockCount = 123; const wrapper = setup.mount(); - expect(wrapper.find('.pipeline-nodelist__row__count').text()).toBe( + expect(wrapper.find('.node-list-row__count').text()).toBe( mockCount.toString() ); }); @@ -128,9 +128,7 @@ describe('NodeListRow', () => { it('does not show count if count prop not set', () => { const { props } = setupProps(); const wrapper = setup.mount(); - expect(wrapper.find('.pipeline-nodelist__row__count').exists()).toBe( - false - ); + expect(wrapper.find('.node-list-row__count').exists()).toBe(false); }); describe('focus mode', () => { diff --git a/src/components/node-list/node-list.test.js b/src/components/node-list/node-list.test.js index edceb82879..de1f8e20c2 100644 --- a/src/components/node-list/node-list.test.js +++ b/src/components/node-list/node-list.test.js @@ -59,7 +59,7 @@ describe('NodeList', () => { const search = () => wrapper.find('.search-input__field'); search().simulate('change', { target: { value: searchText } }); const nodeList = wrapper.find( - '.pipeline-nodelist__elements-panel .pipeline-nodelist__row' + '.pipeline-nodelist__elements-panel .node-list-row' ); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -101,9 +101,7 @@ describe('NodeList', () => { // Re-find elements from root each time to see updates const search = () => wrapper.find('.search-input__field'); const nodeList = () => - wrapper.find( - '.pipeline-nodelist__elements-panel .pipeline-nodelist__row' - ); + wrapper.find('.pipeline-nodelist__elements-panel .node-list-row'); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -148,9 +146,7 @@ describe('NodeList', () => { // Re-find elements from root each time to see updates const search = () => wrapper.find('.search-input__field'); const nodeList = () => - wrapper.find( - '.pipeline-nodelist__elements-panel .pipeline-nodelist__row' - ); + wrapper.find('.pipeline-nodelist__elements-panel .node-list-row'); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -192,7 +188,7 @@ describe('NodeList', () => { const elements = (wrapper) => wrapper .find('.MuiTreeItem-label') - .find('.pipeline-nodelist__row') + .find('.node-list-row') .map((row) => [row.prop('title')]); it('shows full node names when pretty name is turned off', () => { @@ -233,10 +229,10 @@ describe('NodeList', () => { describe('checkboxes on tag filter items', () => { const checkboxByName = (wrapper, text) => - wrapper.find(`.pipeline-nodelist__row__checkbox[name="${text}"]`); + wrapper.find(`.node-list-row__checkbox[name="${text}"]`); const rowByName = (wrapper, text) => - wrapper.find(`.pipeline-nodelist__row[title="${text}"]`); + wrapper.find(`.node-list-row[title="${text}"]`); const changeRows = (wrapper, names, checked) => names.forEach((name) => @@ -248,10 +244,10 @@ describe('NodeList', () => { const elements = (wrapper) => wrapper .find('.MuiTreeItem-label') - .find('.pipeline-nodelist__row') + .find('.node-list-row') .map((row) => [ row.prop('title'), - !row.hasClass('pipeline-nodelist__row--disabled'), + !row.hasClass('node-list-row--disabled'), ]); const elementsEnabled = (wrapper) => { @@ -331,7 +327,7 @@ describe('NodeList', () => { ); - const uncheckedClass = 'pipeline-nodelist__row--unchecked'; + const uncheckedClass = 'node-list-row--unchecked'; expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( true @@ -391,7 +387,7 @@ describe('NodeList', () => { ); const nodeList = wrapper.find( - '.pipeline-nodelist__list--nested .pipeline-nodelist__row' + '.pipeline-nodelist__list--nested .node-list-row' ); // const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -404,7 +400,7 @@ describe('NodeList', () => { ); - const nodeList = wrapper.find('.pipeline-nodelist__row__text--tree'); + const nodeList = wrapper.find('.node-list-row__text--tree'); const modularPipelinesTree = getModularPipelinesTree( mockState.spaceflights ); @@ -444,16 +440,16 @@ describe('NodeList', () => { ); // this needs to be the 3rd element as the first 2 elements are modular pipelines rows which does not apply the '--active' class - const nodeRow = () => wrapper.find('.pipeline-nodelist__row').at(3); + const nodeRow = () => wrapper.find('.node-list-row').at(3); it('handles mouseenter events', () => { nodeRow().simulate('mouseenter'); - expect(nodeRow().hasClass('pipeline-nodelist__row--active')).toBe(true); + expect(nodeRow().hasClass('node-list-row--active')).toBe(true); }); it('handles mouseleave events', () => { nodeRow().simulate('mouseleave'); - expect(nodeRow().hasClass('pipeline-nodelist__row--active')).toBe(false); + expect(nodeRow().hasClass('node-list-row--active')).toBe(false); }); }); @@ -463,7 +459,7 @@ describe('NodeList', () => { ); - const checkbox = () => wrapper.find('.pipeline-nodelist__row input').at(4); + const checkbox = () => wrapper.find('.node-list-row input').at(4); it('handles toggle off event', () => { checkbox().simulate('change', { @@ -507,7 +503,7 @@ describe('NodeList', () => { it('After applying any filter filter button should not be disabled', () => { const nodeTypeFilter = wrapper.find( - `.pipeline-nodelist__row__checkbox[name="Datasets"]` + `.node-list-row__checkbox[name="Datasets"]` ); nodeTypeFilter.simulate('click'); From 976ff98a03576321e9060d5ec8b46b920f8e2bc8 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 15 Oct 2024 17:01:24 +0200 Subject: [PATCH 03/46] update the rest of the classnames Signed-off-by: Huong Nguyen --- src/components/node-list/node-list-row.js | 48 +++++++++---------- .../node-list/styles/_row-toggle.scss | 44 ++++++++--------- src/components/node-list/styles/_row.scss | 2 +- 3 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js index b145dec2d7..91cb0eb98e 100644 --- a/src/components/node-list/node-list-row.js +++ b/src/components/node-list/node-list-row.js @@ -144,13 +144,13 @@ const NodeListRow = memo( )} {VisibilityIcon && ( - + )} {FocusIcon && ( - + )} {children} diff --git a/src/components/node-list/styles/node-list.scss b/src/components/node-list/styles/node-list.scss index 3d45c4f370..6ec3c9bedb 100644 --- a/src/components/node-list/styles/node-list.scss +++ b/src/components/node-list/styles/node-list.scss @@ -4,7 +4,6 @@ @use './panels'; @use './row'; @use './row-label'; -@use './row-toggle'; @use './section'; @use './variables'; From 988ca60ad968765df6fbc3cee11d8bb4f21af832 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 15 Oct 2024 21:35:45 +0200 Subject: [PATCH 05/46] tidy up code for toggle component Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.js | 67 +++++++++++-------- .../node-list-row-toggle.scss | 46 ++++++------- src/components/node-list/node-list-row.js | 2 + 3 files changed, 62 insertions(+), 53 deletions(-) diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.js b/src/components/node-list-row-toggle/node-list-row-toggle.js index 37d9a6a2f7..ab336b390d 100644 --- a/src/components/node-list-row-toggle/node-list-row-toggle.js +++ b/src/components/node-list-row-toggle/node-list-row-toggle.js @@ -1,65 +1,74 @@ import React from 'react'; import classnames from 'classnames'; +import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; import './node-list-row-toggle.scss'; export const NodeListRowToggle = ({ allUnchecked, - isParent, + className, disabled, focusChecked, IconComponent, id, isChecked, + isParent, kind, name, onChange, onToggleHoveredFocusMode, selected, }) => { + const handleMouseHover = (isEntering) => + onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); + + const iconClassNames = classnames( + className, + 'node-list-row-toggle--icon', + `node-list-row-toggle--icon--kind-${kind}`, { + 'node-list-row-toggle--icon--parent': isParent, + 'node-list-row-toggle--icon--child': !isParent, + 'node-list-row-toggle--icon--checked': isChecked, + 'node-list-row-toggle--icon--unchecked': !isChecked, + 'node-list-row-toggle--icon--all-unchecked': allUnchecked, + 'node-list-row-toggle--icon--focus-checked': focusChecked, + } + ); + + const labelClassNames = classnames( + 'node-list-row-toggle', + `node-list-row-toggle--kind-${kind}`, { + 'node-list-row-toggle--disabled': disabled, + 'node-list-row-toggle--selected': selected, + } + ); + + const dataTestValue = getDataTestAttribute('node-list-row-toggle', kind === 'focus' ? 'focusMode' : 'visible', name); + return ( - ) + ); } \ No newline at end of file diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.scss b/src/components/node-list-row-toggle/node-list-row-toggle.scss index 0b80bee0f6..44f7a3b186 100644 --- a/src/components/node-list-row-toggle/node-list-row-toggle.scss +++ b/src/components/node-list-row-toggle/node-list-row-toggle.scss @@ -2,9 +2,7 @@ @use '../../styles/variables' as colors; @use '../node-list/styles/variables'; -// --- Toggle ---// - -.node-list-row__toggle { +.node-list-row-toggle { cursor: pointer; &--kind-element { @@ -26,7 +24,7 @@ variables.$row-selected-dark ); -.node-list-row__toggle--selected::before { +.node-list-row-toggle--selected::before { opacity: 1; } @@ -36,7 +34,7 @@ // --- Toggle icon ---// -.node-list-row__toggle-icon { +.node-list-row-toggle--icon { width: variables.$toggle-icon-size; height: variables.$toggle-icon-size; padding: variables.$toggle-icon-padding; @@ -71,7 +69,7 @@ $element-icon-opacity-0: 0; $element-icon-opacity-1: 0.55; $element-icon-opacity-2: 1; -.node-list-row__toggle-icon--kind-element { +.node-list-row-toggle--icon--kind-element { // Change opacity on the SVG's child elements instead, in order to // maintain 100% opacity outline on parent SVG on keyboard focus > * { @@ -83,7 +81,7 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.node-list-row__toggle-icon--focus-checked { + &.node-list-row-toggle--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -101,14 +99,14 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.node-list-row__toggle-icon--checked { + &.node-list-row-toggle--icon--checked { > * { opacity: $element-icon-opacity-2; } } } - &.node-list-row__toggle-icon--focus-checked { + &.node-list-row-toggle--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -139,18 +137,18 @@ $filter-icon-opacity-1: 0.55; $filter-icon-opacity-2: 0.9; $filter-icon-opacity-3: 1; -.node-list-row__toggle-icon--kind-filter { +.node-list-row-toggle--icon--kind-filter { // Change opacity on the SVG's child elements instead, in order to // maintain 100% opacity outline on parent SVG on keyboard focus > * { opacity: $filter-icon-opacity-1; } - .pipeline-nodelist__heading &.node-list-row__toggle-icon--all-unchecked > * { + .pipeline-nodelist__heading &.node-list-row-toggle--icon--all-unchecked > * { opacity: $filter-icon-opacity-0; } - &.node-list-row__toggle-icon--all-unchecked { + &.node-list-row-toggle--icon--all-unchecked { > * { opacity: $filter-icon-opacity-1; } @@ -163,7 +161,7 @@ $filter-icon-opacity-3: 1; } .node-list-row:hover & { - &.node-list-row__toggle-icon--parent:hover { + &.node-list-row-toggle--icon--parent:hover { > * { opacity: $filter-icon-opacity-2; } @@ -171,7 +169,7 @@ $filter-icon-opacity-3: 1; } .node-list-row & { - &.node-list-row__toggle-icon--checked { + &.node-list-row-toggle--icon--checked { > * { opacity: $filter-icon-opacity-2; } @@ -179,8 +177,8 @@ $filter-icon-opacity-3: 1; } .node-list-row:hover & { - &.node-list-row__toggle-icon--child { - &.node-list-row__toggle-icon--checked { + &.node-list-row-toggle--icon--child { + &.node-list-row-toggle--icon--checked { > * { opacity: $filter-icon-opacity-3; } @@ -189,8 +187,8 @@ $filter-icon-opacity-3: 1; } .node-list-row & { - &.node-list-row__toggle-icon--parent:hover { - &.node-list-row__toggle-icon--checked { + &.node-list-row-toggle--icon--parent:hover { + &.node-list-row-toggle--icon--checked { > * { opacity: $filter-icon-opacity-3; } @@ -203,7 +201,7 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-2; } - &.node-list-row__toggle-icon--checked { + &.node-list-row-toggle--icon--checked { > * { opacity: $filter-icon-opacity-3; } @@ -213,19 +211,19 @@ $filter-icon-opacity-3: 1; // --- Toggle (kind=filter) icon fills and strokes ---// -.node-list-row__toggle-icon--kind-filter { - &.node-list-row__toggle-icon--checked { +.node-list-row-toggle--icon--kind-filter { + &.node-list-row-toggle--icon--checked { fill: var(--color-nodelist-filter-indicator-on); stroke: var(--color-nodelist-filter-indicator-on); } - &.node-list-row__toggle-icon--unchecked { + &.node-list-row-toggle--icon--unchecked { fill: none; stroke: var(--color-nodelist-filter-indicator-off); } - .node-list-row:hover &.node-list-row__toggle-icon--all-unchecked, - &.node-list-row__toggle-icon--parent { + .node-list-row:hover &.node-list-row-toggle--icon--all-unchecked, + &.node-list-row-toggle--icon--parent { fill: colors.$blue-300; stroke: colors.$blue-300; } diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js index c3814d3839..c1a581358d 100644 --- a/src/components/node-list/node-list-row.js +++ b/src/components/node-list/node-list-row.js @@ -144,6 +144,7 @@ const NodeListRow = memo( {VisibilityIcon && ( Date: Wed, 16 Oct 2024 11:43:31 +0200 Subject: [PATCH 06/46] update classnames in tests Signed-off-by: Huong Nguyen --- cypress/tests/ui/flowchart/menu.cy.js | 26 +++++++++---------- .../node-list/styles/_row-label.scss | 4 +-- tools/test-lib/react-app/app.test.js | 5 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cypress/tests/ui/flowchart/menu.cy.js b/cypress/tests/ui/flowchart/menu.cy.js index deb6d38f81..cba8f32528 100644 --- a/cypress/tests/ui/flowchart/menu.cy.js +++ b/cypress/tests/ui/flowchart/menu.cy.js @@ -41,7 +41,7 @@ describe('Flowchart Menu', () => { }); // Pipeline Label in the Menu - cy.get('.pipeline-nodelist__row__label') + cy.get('.node-list-row__label') .first() .invoke('text') .should((pipelineLabel) => { @@ -57,7 +57,7 @@ describe('Flowchart Menu', () => { cy.get('.search-input__field').type(searchInput, { force: true }); // Pipeline Label in the Menu - cy.get('.pipeline-nodelist__row__label') + cy.get('.node-list-row__label') .first() .invoke('text') .should((pipelineLabel) => { @@ -72,7 +72,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `.MuiTreeItem-label > .pipeline-nodelist__row > [data-test=nodelist-data-${nodeToClickText}]` + `.MuiTreeItem-label > .node-list-row > [data-test=nodelist-data-${nodeToClickText}]` ) .should('exist') .as('nodeToClick'); @@ -91,7 +91,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `.MuiTreeItem-label > .pipeline-nodelist__row > [data-test=nodelist-data-${nodeToHighlightText}]` + `.MuiTreeItem-label > .node-list-row > [data-test=nodelist-data-${nodeToHighlightText}]` ) .should('exist') .as('nodeToHighlight'); @@ -108,7 +108,7 @@ describe('Flowchart Menu', () => { const nodeToToggleText = 'Companies'; // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${nodeToToggleText}]`, { + cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`, { timeout: 5000, }).as('nodeToToggle'); @@ -121,7 +121,7 @@ describe('Flowchart Menu', () => { // Assert after action cy.__checkForText__( - `[data-test=nodelist-data-${nodeToToggleText}] > .pipeline-nodelist__row__label--faded`, + `[data-test=nodelist-data-${nodeToToggleText}] > .node-list-row__label--faded`, nodeToToggleText ); cy.get('.pipeline-node__text').should('not.contain', nodeToToggleText); @@ -137,7 +137,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `[for=${nodeToFocusText}-focus] > .pipeline-nodelist__row__icon` + `[for=${nodeToFocusText}-focus] > .node-list-row__icon` ).click(); // Assert after action @@ -161,14 +161,14 @@ describe('Flowchart Menu', () => { const visibleRowLabel = 'Companies'; // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${nodeToToggleText}]`).as( + cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`).as( 'nodeToToggle' ); // Assert before action cy.get('@nodeToToggle').should('be.checked'); cy.get( - `[data-test=nodelist-data-${visibleRowLabel}] > .pipeline-nodelist__row__label` + `[data-test=nodelist-data-${visibleRowLabel}] > .node-list-row__label` ) .should('not.have.class', 'pipeline-nodelist__row__label--faded') .should('not.have.class', 'pipeline-nodelist__row__label--disabled'); @@ -178,7 +178,7 @@ describe('Flowchart Menu', () => { // Assert after action cy.get( - `[data-test=nodelist-data-${visibleRowLabel}] > .pipeline-nodelist__row__label` + `[data-test=nodelist-data-${visibleRowLabel}] > .node-list-row__label` ) .should('have.class', 'pipeline-nodelist__row__label--faded') .should('have.class', 'pipeline-nodelist__row__label--disabled'); @@ -188,7 +188,7 @@ describe('Flowchart Menu', () => { const nodeToToggleText = 'Parameters'; // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${nodeToToggleText}]`).as( + cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`).as( 'nodeToToggle' ); @@ -207,7 +207,7 @@ describe('Flowchart Menu', () => { cy.visit(`/?tags=${visibleRowLabel}`); // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${visibleRowLabel}]`).as( + cy.get(`.node-list-row__checkbox[name=${visibleRowLabel}]`).as( 'nodeToToggle' ); @@ -220,7 +220,7 @@ describe('Flowchart Menu', () => { cy.visit('/?types=datasets'); // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${visibleRowLabel}]`).as( + cy.get(`.node-list-row__checkbox[name=${visibleRowLabel}]`).as( 'nodeToToggle' ); diff --git a/src/components/node-list/styles/_row-label.scss b/src/components/node-list/styles/_row-label.scss index 3ec8971ca9..e90674d45e 100644 --- a/src/components/node-list/styles/_row-label.scss +++ b/src/components/node-list/styles/_row-label.scss @@ -75,12 +75,12 @@ opacity: 0.75; user-select: none; - .pipeline-nodelist__row--unchecked & { + .node-list-row--unchecked & { opacity: 0.55; } } -.pipeline-nodelist__row--unchecked { +.node-list-row--unchecked { // Fade row text when unchecked .node-list-row__label--kind-filter { opacity: 0.55; diff --git a/tools/test-lib/react-app/app.test.js b/tools/test-lib/react-app/app.test.js index 07354db84b..38ec6aa26a 100644 --- a/tools/test-lib/react-app/app.test.js +++ b/tools/test-lib/react-app/app.test.js @@ -17,9 +17,8 @@ describe('lib-test', () => { */ const testFirstNodeNameMatch = (container, key) => { const firstNodeName = container - .querySelector('.pipeline-nodelist__row') - .querySelector('.pipeline-nodelist__row__text--tree') - .querySelector('.pipeline-nodelist__row__label') + .querySelector('.node-list-row') + .querySelector('.node-list-row__label') .textContent.trim(); const modularPipelinesTree = dataSources[key]().modular_pipelines; From abf00c8ef2c9f1831e68c5963cfc3d81743f4e1f Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 16 Oct 2024 11:59:15 +0200 Subject: [PATCH 07/46] simplify the css Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.scss | 51 ++++--------------- 1 file changed, 9 insertions(+), 42 deletions(-) diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.scss b/src/components/node-list-row-toggle/node-list-row-toggle.scss index 44f7a3b186..acaea7fef5 100644 --- a/src/components/node-list-row-toggle/node-list-row-toggle.scss +++ b/src/components/node-list-row-toggle/node-list-row-toggle.scss @@ -144,13 +144,10 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-1; } + &.node-list-row-toggle--icon--all-unchecked, .pipeline-nodelist__heading &.node-list-row-toggle--icon--all-unchecked > * { - opacity: $filter-icon-opacity-0; - } - - &.node-list-row-toggle--icon--all-unchecked { > * { - opacity: $filter-icon-opacity-1; + opacity: $filter-icon-opacity-0; } } @@ -158,60 +155,30 @@ $filter-icon-opacity-3: 1; > * { opacity: $filter-icon-opacity-1; } - } - - .node-list-row:hover & { - &.node-list-row-toggle--icon--parent:hover { - > * { - opacity: $filter-icon-opacity-2; - } - } - } - .node-list-row & { - &.node-list-row-toggle--icon--checked { + &.node-list-row-toggle--icon--parent:hover, + &.node-list-row-toggle--icon--checked, + &.node-list-row-toggle--icon--child.node-list-row-toggle--icon--checked { > * { - opacity: $filter-icon-opacity-2; - } - } - } - - .node-list-row:hover & { - &.node-list-row-toggle--icon--child { - &.node-list-row-toggle--icon--checked { - > * { - opacity: $filter-icon-opacity-3; - } - } - } - } - - .node-list-row & { - &.node-list-row-toggle--icon--parent:hover { - &.node-list-row-toggle--icon--checked { - > * { - opacity: $filter-icon-opacity-3; - } + opacity: $filter-icon-opacity-2; // Increase opacity for checked or parent hover } } } [data-whatintent='keyboard'] input:focus + & { > * { - opacity: $filter-icon-opacity-2; + opacity: $filter-icon-opacity-2; // Increase opacity on keyboard focus } &.node-list-row-toggle--icon--checked { > * { - opacity: $filter-icon-opacity-3; + opacity: $filter-icon-opacity-3; // Further increase for checked on focus } } } -} // --- Toggle (kind=filter) icon fills and strokes ---// -.node-list-row-toggle--icon--kind-filter { &.node-list-row-toggle--icon--checked { fill: var(--color-nodelist-filter-indicator-on); stroke: var(--color-nodelist-filter-indicator-on); @@ -227,4 +194,4 @@ $filter-icon-opacity-3: 1; fill: colors.$blue-300; stroke: colors.$blue-300; } -} +} \ No newline at end of file From 3c2d623de3ad7b908fe3790498321f70b3ef3845 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 16 Oct 2024 14:20:30 +0200 Subject: [PATCH 08/46] add tests for node-list-row-toggle Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.test.js | 44 +++++++++++++++++++ .../node-list/node-list-row.test.js | 22 ---------- 2 files changed, 44 insertions(+), 22 deletions(-) create mode 100644 src/components/node-list-row-toggle/node-list-row-toggle.test.js diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.test.js b/src/components/node-list-row-toggle/node-list-row-toggle.test.js new file mode 100644 index 0000000000..a4e117afe3 --- /dev/null +++ b/src/components/node-list-row-toggle/node-list-row-toggle.test.js @@ -0,0 +1,44 @@ +import React from 'react'; +import { shallow } from 'enzyme'; +import { NodeListRowToggle } from './node-list-row-toggle'; + +describe('NodeListRowToggle', () => { + const baseProps = { + name: 'Test Node', + onChange: jest.fn(), + onToggleHoveredFocusMode: jest.fn(), + }; + + it('applies "all-unchecked" class when allUnchecked is true', () => { + const props = { ...baseProps, allUnchecked: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe(true); + }); + + it('applies "disabled" class when disabled is true', () => { + const props = { ...baseProps, disabled: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(true); + }); + + it('applies "checked" class when isChecked is true', () => { + const props = { ...baseProps, isChecked: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(true); + }); + + it('applies "parent" class when isParent is true', () => { + const props = { ...baseProps, isParent: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(true); + }); + + it('applies correct class for kind prop', () => { + const kinds = ['modularPipeline', 'data', 'task']; + kinds.forEach(kind => { + const props = { ...baseProps, kind }; + const wrapper = shallow(); + expect(wrapper.hasClass(`node-list-row-toggle--kind-${kind}`)).toBe(true); + }); + }); +}); \ No newline at end of file diff --git a/src/components/node-list/node-list-row.test.js b/src/components/node-list/node-list-row.test.js index 5414a6b20b..61e3fa96fc 100644 --- a/src/components/node-list/node-list-row.test.js +++ b/src/components/node-list/node-list-row.test.js @@ -132,28 +132,6 @@ describe('NodeListRow', () => { }); describe('focus mode', () => { - it('sets the focus toggle to the checked mode when the row is selected for focus mode', () => { - const { props } = setupProps(); - const wrapper = setup.mount( - - ); - - expect( - wrapper.find('.pipeline-row__toggle-icon--focus-checked').exists() - ).toBe(true); - }); - - it('hides the visibility toggle when the row is selected for focus mode', () => { - const { props } = setupProps(); - const wrapper = setup.mount( - - ); - - expect(wrapper.find('.pipeline-row__toggle--disabled').exists()).toBe( - true - ); - }); - it('switches the visibility toggle from hide to show when the row is selected for focus mode', () => { const { props } = setupProps(); const wrapper = setup.mount( From 479701e35dc2587e2754195d11c3414e3c1db3e2 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 16 Oct 2024 16:21:01 +0200 Subject: [PATCH 09/46] remove handleToggle on VisibilityIcon Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.js | 130 +++++++++--------- src/components/node-list/node-list-row.js | 7 +- 2 files changed, 71 insertions(+), 66 deletions(-) diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.js b/src/components/node-list-row-toggle/node-list-row-toggle.js index ab336b390d..f88db295f1 100644 --- a/src/components/node-list-row-toggle/node-list-row-toggle.js +++ b/src/components/node-list-row-toggle/node-list-row-toggle.js @@ -5,70 +5,76 @@ import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; import './node-list-row-toggle.scss'; export const NodeListRowToggle = ({ - allUnchecked, - className, - disabled, - focusChecked, - IconComponent, - id, - isChecked, - isParent, - kind, - name, - onChange, - onToggleHoveredFocusMode, - selected, + allUnchecked, + className, + disabled, + focusChecked, + IconComponent, + id, + isChecked, + isParent, + kind, + name, + onChange, + onToggleHoveredFocusMode, + selected, }) => { - const handleMouseHover = (isEntering) => - onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); - - const iconClassNames = classnames( - className, - 'node-list-row-toggle--icon', - `node-list-row-toggle--icon--kind-${kind}`, { - 'node-list-row-toggle--icon--parent': isParent, - 'node-list-row-toggle--icon--child': !isParent, - 'node-list-row-toggle--icon--checked': isChecked, - 'node-list-row-toggle--icon--unchecked': !isChecked, - 'node-list-row-toggle--icon--all-unchecked': allUnchecked, - 'node-list-row-toggle--icon--focus-checked': focusChecked, + const handleMouseHover = (isEntering) => + onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); + + const iconClassNames = classnames( + className, + 'node-list-row-toggle--icon', + `node-list-row-toggle--icon--kind-${kind}`, + { + 'node-list-row-toggle--icon--parent': isParent, + 'node-list-row-toggle--icon--child': !isParent, + 'node-list-row-toggle--icon--checked': isChecked, + 'node-list-row-toggle--icon--unchecked': !isChecked, + 'node-list-row-toggle--icon--all-unchecked': allUnchecked, + 'node-list-row-toggle--icon--focus-checked': focusChecked, } - ); + ); - const labelClassNames = classnames( + const labelClassNames = classnames( 'node-list-row-toggle', - `node-list-row-toggle--kind-${kind}`, { - 'node-list-row-toggle--disabled': disabled, - 'node-list-row-toggle--selected': selected, + `node-list-row-toggle--kind-${kind}`, + { + 'node-list-row-toggle--disabled': disabled, + 'node-list-row-toggle--selected': selected, } - ); - - const dataTestValue = getDataTestAttribute('node-list-row-toggle', kind === 'focus' ? 'focusMode' : 'visible', name); - - return ( - - ); -} \ No newline at end of file + ); + + const dataTestValue = getDataTestAttribute( + 'node-list-row-toggle', + kind === 'focus' ? 'focusMode' : 'visible', + name + ); + + return ( + + ); +}; diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js index c1a581358d..8d1a7fa079 100644 --- a/src/components/node-list/node-list-row.js +++ b/src/components/node-list/node-list-row.js @@ -142,7 +142,7 @@ const NodeListRow = memo( )} {VisibilityIcon && ( - )} {FocusIcon && ( - Date: Wed, 16 Oct 2024 18:12:27 +0200 Subject: [PATCH 10/46] remove redux from node-list-row Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.js | 3 +- src/components/node-list/index.js | 11 +++- src/components/node-list/node-list-group.js | 2 +- .../node-list/node-list-row-list.js | 2 +- src/components/node-list/node-list-row.js | 55 ++++++------------- .../node-list/node-list-row.test.js | 2 +- .../node-list/node-list-tree-item.js | 4 +- 7 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.js b/src/components/node-list-row-toggle/node-list-row-toggle.js index f88db295f1..6c17a51c45 100644 --- a/src/components/node-list-row-toggle/node-list-row-toggle.js +++ b/src/components/node-list-row-toggle/node-list-row-toggle.js @@ -18,6 +18,7 @@ export const NodeListRowToggle = ({ onChange, onToggleHoveredFocusMode, selected, + dataIconType, }) => { const handleMouseHover = (isEntering) => onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); @@ -68,7 +69,7 @@ export const NodeListRowToggle = ({ disabled={disabled} name={name} onChange={onChange} - data--icon-type={kind} + data-icon-type={dataIconType} /> ({ onToggleFocusMode: (modularPipeline) => { dispatch(toggleFocusMode(modularPipeline)); }, + onToggleHoveredFocusMode: (active) => { + dispatch(toggleHoveredFocusMode(active)); + }, onResetSlicePipeline: () => { dispatch(resetSlicePipeline()); }, diff --git a/src/components/node-list/node-list-group.js b/src/components/node-list/node-list-group.js index 9b54a2d72b..4ff2bc5164 100644 --- a/src/components/node-list/node-list-group.js +++ b/src/components/node-list/node-list-group.js @@ -1,6 +1,6 @@ import React from 'react'; import classnames from 'classnames'; -import NodeListRow from './node-list-row'; +import { NodeListRow } from './node-list-row'; import NodeRowList from './node-list-row-list'; export const NodeListGroup = ({ diff --git a/src/components/node-list/node-list-row-list.js b/src/components/node-list/node-list-row-list.js index 4566fbaafc..8886a9fe07 100644 --- a/src/components/node-list/node-list-row-list.js +++ b/src/components/node-list/node-list-row-list.js @@ -1,6 +1,6 @@ import React from 'react'; import modifiers from '../../utils/modifiers'; -import NodeListRow, { nodeListRowHeight } from './node-list-row'; +import { NodeListRow, nodeListRowHeight } from './node-list-row'; import LazyList from '../lazy-list'; const NodeRowList = ({ diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js index 8d1a7fa079..8132e29ae4 100644 --- a/src/components/node-list/node-list-row.js +++ b/src/components/node-list/node-list-row.js @@ -1,18 +1,14 @@ import React, { memo } from 'react'; -import { connect } from 'react-redux'; import classnames from 'classnames'; import { changed, replaceAngleBracketMatches } from '../../utils'; import NodeIcon from '../icons/node-icon'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; import FocusModeIcon from '../icons/focus-mode'; -import { getNodeActive } from '../../selectors/nodes'; -import { toggleHoveredFocusMode } from '../../actions'; import { NodeListRowToggle } from '../node-list-row-toggle/node-list-row-toggle'; // The exact fixed height of a row as measured by getBoundingClientRect() export const nodeListRowHeight = 32; - /** * Returns `true` if there are no props changes, therefore the last render can be reused. * Performance: Checks only the minimal set of props known to change after first render. @@ -37,36 +33,36 @@ const shouldMemo = (prevProps, nextProps) => nextProps ); -const NodeListRow = memo( +export const NodeListRow = memo( ({ - container: Container = 'div', active, - checked, allUnchecked, + checked, children, + container: Container = 'div', + count, disabled, faded, focused, - visible, + focusModeIcon = FocusModeIcon, + highlight, + icon, id, + invisibleIcon = InvisibleIcon, + isSlicingPipelineApplied, + kind, label, - count, name, - kind, - onMouseEnter, - onMouseLeave, onChange, onClick, + onMouseEnter, + onMouseLeave, + onToggleHoveredFocusMode, + rowType, selected, - highlight, - isSlicingPipelineApplied, type, - icon, + visible, visibleIcon = VisibleIcon, - invisibleIcon = InvisibleIcon, - focusModeIcon = FocusModeIcon, - rowType, - onToggleHoveredFocusMode, }) => { const isModularPipeline = type === 'modularPipeline'; const FocusIcon = isModularPipeline ? focusModeIcon : null; @@ -146,7 +142,7 @@ const NodeListRow = memo( allUnchecked={allUnchecked} className={'node-list-row__icon'} isParent={Boolean(children)} - disabled={disabled} + disabled={isModularPipeline ? focused : disabled} focusChecked={isModularPipeline ? false : focused} IconComponent={VisibilityIcon} id={id} @@ -165,13 +161,14 @@ const NodeListRow = memo( disabled={disabled} focusChecked={focused} IconComponent={FocusIcon} - id={id + '-focus'} + id={`${id}-focus`} isChecked={isChecked} kind={kind} name={name} onChange={onChange} onToggleHoveredFocusMode={onToggleHoveredFocusMode} selected={selected} + dataIconType="focus" /> )} {children} @@ -180,19 +177,3 @@ const NodeListRow = memo( }, shouldMemo ); - -export const mapDispatchToProps = (dispatch) => ({ - onToggleHoveredFocusMode: (active) => { - dispatch(toggleHoveredFocusMode(active)); - }, -}); - -export const mapStateToProps = (state, ownProps) => ({ - ...ownProps, - active: - typeof ownProps.active !== 'undefined' - ? ownProps.active - : getNodeActive(state)[ownProps.id] || false, -}); - -export default connect(mapStateToProps, mapDispatchToProps)(NodeListRow); diff --git a/src/components/node-list/node-list-row.test.js b/src/components/node-list/node-list-row.test.js index 61e3fa96fc..95aa756021 100644 --- a/src/components/node-list/node-list-row.test.js +++ b/src/components/node-list/node-list-row.test.js @@ -1,5 +1,5 @@ import React from 'react'; -import NodeListRow, { mapStateToProps } from './node-list-row'; +import { NodeListRow, mapStateToProps } from './node-list-row'; import { getNodeData } from '../../selectors/nodes'; import { setup, mockState } from '../../utils/state.mock'; diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index 5a08c0ca25..83c258b9dc 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -2,7 +2,7 @@ import React from 'react'; import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; -import NodeListRow from './node-list-row'; +import { NodeListRow } from './node-list-row'; const arrowIconColor = '#8e8e90'; @@ -12,6 +12,7 @@ const NodeListTreeItem = ({ onItemMouseEnter, onItemMouseLeave, onItemChange, + onToggleHoveredFocusMode, children, isSlicingPipelineApplied, }) => ( @@ -50,6 +51,7 @@ const NodeListTreeItem = ({ onChange={(e) => onItemChange(data, !e.target.checked, e.target.dataset.iconType) } + onToggleHoveredFocusMode={onToggleHoveredFocusMode} rowType="tree" focused={data.focused} /> From ca8b72c90d38495b161b6586d0e9009a47d51454 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 10:48:22 +0200 Subject: [PATCH 11/46] split node-list-row into row and filter-row Signed-off-by: Huong Nguyen --- src/components/node-list/filter-row.js | 83 ++++++++++ src/components/node-list/node-list-group.js | 6 +- .../node-list/node-list-row-list.js | 4 +- .../node-list/node-list-tree-item.js | 4 +- src/components/node-list/node-list-tree.js | 3 + src/components/node-list/node-list.js | 2 + src/components/node-list/row.js | 143 ++++++++++++++++++ 7 files changed, 238 insertions(+), 7 deletions(-) create mode 100755 src/components/node-list/filter-row.js create mode 100644 src/components/node-list/row.js diff --git a/src/components/node-list/filter-row.js b/src/components/node-list/filter-row.js new file mode 100755 index 0000000000..347dc0ec15 --- /dev/null +++ b/src/components/node-list/filter-row.js @@ -0,0 +1,83 @@ +import React from 'react'; +import classnames from 'classnames'; +import { replaceAngleBracketMatches } from '../../utils'; +import VisibleIcon from '../icons/visible'; +import InvisibleIcon from '../icons/invisible'; +import { NodeListRowToggle } from '../node-list-row-toggle/node-list-row-toggle'; + +// The exact fixed height of a row as measured by getBoundingClientRect() +export const nodeListRowHeight = 32; +export const FilterRow = ({ + allUnchecked, + checked, + children, + container: Container = 'div', + count, + id, + invisibleIcon = InvisibleIcon, + kind, + label, + name, + onChange, + onClick, + onMouseEnter, + onMouseLeave, + visible, + visibleIcon = VisibleIcon, +}) => { + const VisibilityIcon = checked ? visibleIcon : invisibleIcon; + + return ( + + + + {count} + + {VisibilityIcon && ( + + )} + {children} + + ); +}; diff --git a/src/components/node-list/node-list-group.js b/src/components/node-list/node-list-group.js index 4ff2bc5164..11d94ea4f6 100644 --- a/src/components/node-list/node-list-group.js +++ b/src/components/node-list/node-list-group.js @@ -1,6 +1,6 @@ import React from 'react'; import classnames from 'classnames'; -import { NodeListRow } from './node-list-row'; +import { FilterRow } from './filter-row'; import NodeRowList from './node-list-row-list'; export const NodeListGroup = ({ @@ -35,7 +35,7 @@ export const NodeListGroup = ({ )} >

- onToggleCollapsed(id)} /> - +

{items.slice(start, end).map((item) => ( - } expandIcon={} label={ - { + const isModularPipeline = type === 'modularPipeline'; + const FocusIcon = isModularPipeline ? focusModeIcon : null; + const isChecked = isModularPipeline ? checked || focused : checked; + const VisibilityIcon = isChecked ? visibleIcon : invisibleIcon; + const isButton = onClick && kind !== 'filter'; + const TextButton = isButton ? 'button' : 'div'; + + return ( + + + + + + {VisibilityIcon && ( + + )} + {FocusIcon && ( + + )} + {children} + + ); +}; From 65382f8800365c61b67ffc6dc50d8d657dc7369d Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 11:41:29 +0200 Subject: [PATCH 12/46] rename toggle icon component Signed-off-by: Huong Nguyen --- .../node-list-row-toggle.test.js | 44 --------- src/components/node-list/filter-row.js | 4 +- src/components/node-list/node-list-row.js | 6 +- src/components/node-list/row.js | 10 +- .../toggle-icon/toggle-icon.js} | 6 +- .../toggle-icon/toggle-icon.scss} | 10 +- .../ui/toggle-icon/toggle-icon.test.js | 94 +++++++++++++++++++ 7 files changed, 112 insertions(+), 62 deletions(-) delete mode 100644 src/components/node-list-row-toggle/node-list-row-toggle.test.js rename src/components/{node-list-row-toggle/node-list-row-toggle.js => ui/toggle-icon/toggle-icon.js} (92%) rename src/components/{node-list-row-toggle/node-list-row-toggle.scss => ui/toggle-icon/toggle-icon.scss} (96%) create mode 100644 src/components/ui/toggle-icon/toggle-icon.test.js diff --git a/src/components/node-list-row-toggle/node-list-row-toggle.test.js b/src/components/node-list-row-toggle/node-list-row-toggle.test.js deleted file mode 100644 index a4e117afe3..0000000000 --- a/src/components/node-list-row-toggle/node-list-row-toggle.test.js +++ /dev/null @@ -1,44 +0,0 @@ -import React from 'react'; -import { shallow } from 'enzyme'; -import { NodeListRowToggle } from './node-list-row-toggle'; - -describe('NodeListRowToggle', () => { - const baseProps = { - name: 'Test Node', - onChange: jest.fn(), - onToggleHoveredFocusMode: jest.fn(), - }; - - it('applies "all-unchecked" class when allUnchecked is true', () => { - const props = { ...baseProps, allUnchecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe(true); - }); - - it('applies "disabled" class when disabled is true', () => { - const props = { ...baseProps, disabled: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(true); - }); - - it('applies "checked" class when isChecked is true', () => { - const props = { ...baseProps, isChecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(true); - }); - - it('applies "parent" class when isParent is true', () => { - const props = { ...baseProps, isParent: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(true); - }); - - it('applies correct class for kind prop', () => { - const kinds = ['modularPipeline', 'data', 'task']; - kinds.forEach(kind => { - const props = { ...baseProps, kind }; - const wrapper = shallow(); - expect(wrapper.hasClass(`node-list-row-toggle--kind-${kind}`)).toBe(true); - }); - }); -}); \ No newline at end of file diff --git a/src/components/node-list/filter-row.js b/src/components/node-list/filter-row.js index 347dc0ec15..7c0f76c325 100755 --- a/src/components/node-list/filter-row.js +++ b/src/components/node-list/filter-row.js @@ -3,7 +3,7 @@ import classnames from 'classnames'; import { replaceAngleBracketMatches } from '../../utils'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; -import { NodeListRowToggle } from '../node-list-row-toggle/node-list-row-toggle'; +import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; // The exact fixed height of a row as measured by getBoundingClientRect() export const nodeListRowHeight = 32; @@ -66,7 +66,7 @@ export const FilterRow = ({ {count} {VisibilityIcon && ( - )} {VisibilityIcon && ( - )} {FocusIcon && ( - {VisibilityIcon && ( - )} {FocusIcon && ( - { + const baseProps = { + name: 'Test Node', + onChange: jest.fn(), + onToggleHoveredFocusMode: jest.fn(), + }; + + it('applies "all-unchecked" class when allUnchecked is true', () => { + const props = { ...baseProps, allUnchecked: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe( + true + ); + }); + + it('applies "disabled" class when disabled is true', () => { + const props = { ...baseProps, disabled: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(true); + }); + + it('applies "checked" class when isChecked is true', () => { + const props = { ...baseProps, isChecked: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(true); + }); + + it('applies "parent" class when isParent is true', () => { + const props = { ...baseProps, isParent: true }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(true); + }); + + it('applies correct class for kind prop', () => { + const kinds = ['modularPipeline', 'data', 'task']; + kinds.forEach((kind) => { + const props = { ...baseProps, kind }; + const wrapper = shallow(); + expect(wrapper.hasClass(`node-list-row-toggle--kind-${kind}`)).toBe(true); + }); + }); + + it('does not apply "all-unchecked" class when allUnchecked is false', () => { + const props = { ...baseProps, allUnchecked: false }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe( + false + ); + }); + + it('does not apply "disabled" class when disabled is false', () => { + const props = { ...baseProps, disabled: false }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(false); + }); + + it('does not apply "checked" class when isChecked is false', () => { + const props = { ...baseProps, isChecked: false }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(false); + }); + + it('does not apply "parent" class when isParent is false', () => { + const props = { ...baseProps, isParent: false }; + const wrapper = shallow(); + expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(false); + }); + + it('triggers onChange callback when clicked', () => { + const props = { ...baseProps }; + const wrapper = shallow(); + wrapper.simulate('click'); + expect(props.onChange).toHaveBeenCalled(); + }); + + it('does not trigger onToggleHoveredFocusMode when not provided', () => { + const props = { ...baseProps, onToggleHoveredFocusMode: undefined }; + const wrapper = shallow(); + wrapper.simulate('click'); + // Since onToggleHoveredFocusMode is not provided, it should not throw an error + expect(() => wrapper.simulate('click')).not.toThrow(); + }); + + it('triggers onToggleHoveredFocusMode when provided and clicked', () => { + const props = { ...baseProps }; + const wrapper = shallow(); + wrapper.simulate('click'); + expect(props.onToggleHoveredFocusMode).toHaveBeenCalled(); + }); +}); From 9162c10fad5f28553414c76a960a3d1fec12b71e Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 12:23:24 +0200 Subject: [PATCH 13/46] move row and filter-row to components level Signed-off-by: Huong Nguyen --- .../{node-list => filter-row}/filter-row.js | 22 +++++++++---------- src/components/node-list/node-list-group.js | 2 +- .../node-list/node-list-row-list.js | 2 +- .../node-list/node-list-tree-item.js | 2 +- src/components/{node-list => row}/row.js | 3 --- 5 files changed, 13 insertions(+), 18 deletions(-) rename src/components/{node-list => filter-row}/filter-row.js (85%) rename src/components/{node-list => row}/row.js (97%) diff --git a/src/components/node-list/filter-row.js b/src/components/filter-row/filter-row.js similarity index 85% rename from src/components/node-list/filter-row.js rename to src/components/filter-row/filter-row.js index 7c0f76c325..c66ad53f58 100755 --- a/src/components/node-list/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -65,18 +65,16 @@ export const FilterRow = ({ {count} - {VisibilityIcon && ( - - )} + {children} ); diff --git a/src/components/node-list/node-list-group.js b/src/components/node-list/node-list-group.js index 11d94ea4f6..a90becae78 100644 --- a/src/components/node-list/node-list-group.js +++ b/src/components/node-list/node-list-group.js @@ -1,6 +1,6 @@ import React from 'react'; import classnames from 'classnames'; -import { FilterRow } from './filter-row'; +import { FilterRow } from '../filter-row/filter-row'; import NodeRowList from './node-list-row-list'; export const NodeListGroup = ({ diff --git a/src/components/node-list/node-list-row-list.js b/src/components/node-list/node-list-row-list.js index 1179a87d88..c65716da75 100644 --- a/src/components/node-list/node-list-row-list.js +++ b/src/components/node-list/node-list-row-list.js @@ -1,6 +1,6 @@ import React from 'react'; import modifiers from '../../utils/modifiers'; -import { FilterRow, nodeListRowHeight } from './filter-row'; +import { FilterRow, nodeListRowHeight } from '../filter-row/filter-row'; import LazyList from '../lazy-list'; const NodeRowList = ({ diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index aa88e35bce..3491fe5c79 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -2,7 +2,7 @@ import React from 'react'; import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; -import { Row } from './row'; +import { Row } from '../row/row'; const arrowIconColor = '#8e8e90'; diff --git a/src/components/node-list/row.js b/src/components/row/row.js similarity index 97% rename from src/components/node-list/row.js rename to src/components/row/row.js index 56850b5643..84850e2b1b 100644 --- a/src/components/node-list/row.js +++ b/src/components/row/row.js @@ -12,7 +12,6 @@ export const nodeListRowHeight = 32; export const Row = ({ active, - allUnchecked, checked, children, container: Container = 'div', @@ -107,7 +106,6 @@ export const Row = ({ {VisibilityIcon && ( Date: Thu, 17 Oct 2024 15:20:06 +0200 Subject: [PATCH 14/46] move css to row and filterRow Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 26 +++-- .../filter-row.scss} | 52 +++++----- src/components/node-list/styles/_group.scss | 3 +- .../node-list/styles/node-list.scss | 2 - src/components/row/row.js | 63 +++++------- .../styles/_row.scss => row/row.scss} | 98 ++++++++++++++----- 6 files changed, 145 insertions(+), 99 deletions(-) rename src/components/{node-list/styles/_row-label.scss => filter-row/filter-row.scss} (62%) rename src/components/{node-list/styles/_row.scss => row/row.scss} (50%) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index c66ad53f58..b25db37273 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -5,6 +5,8 @@ import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; +import './filter-row.scss'; + // The exact fixed height of a row as measured by getBoundingClientRect() export const nodeListRowHeight = 32; export const FilterRow = ({ @@ -29,22 +31,18 @@ export const FilterRow = ({ return ( - + {count} {VisibilityIcon && ( * { opacity: 0.3; } @@ -103,8 +95,8 @@ &--active, &--selected, - .node-list-row--visible:hover &, - [data-whatintent='keyboard'] .node-list-row__text:focus & { + .row--visible:hover &, + [data-whatintent='keyboard'] .row__text:focus & { > * { opacity: 1; } @@ -114,3 +106,65 @@ } } } + +.pipeline-nodelist__elements-panel .MuiTreeItem-label { + // Handle MuiTreeItem icon offset for correct width + $icon-offset: 15px + 4px; + + width: calc(100% - #{$icon-offset}); +} + +.row__text { + display: flex; + align-items: center; + + // Fixed with required for overflow elipsis + width: calc(100% - 7em); + margin-right: auto; + padding: variables.$row-padding-y 0 variables.$row-padding-y 0; + color: inherit; + font-size: inherit; + font-family: inherit; + line-height: 1.6; + letter-spacing: inherit; + text-align: inherit; + background: none; + border: none; + border-radius: 0; + box-shadow: none; + cursor: default; + user-select: none; + + &--tree { + padding: variables.$row-padding-y 0 variables.$row-padding-y 1em; + } + + &:focus { + outline: none; + box-shadow: 0 0 0 4px var.$blue-300 inset; + + [data-whatintent='mouse'] & { + box-shadow: none; + } + } +} + +.row__label { + overflow: hidden; + font-size: 1.4em; + white-space: nowrap; + text-overflow: ellipsis; + + &--faded { + opacity: 0.65; + } + + &--disabled { + opacity: 0.3 !important; + } + + b { + color: var(--color-nodelist-highlight); + font-weight: normal; + } +} From d87c2b27bd79fb101bed3c69427bc6f39b6ced74 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 15:23:24 +0200 Subject: [PATCH 15/46] remove node-list-row Signed-off-by: Huong Nguyen --- src/components/node-list/node-list-row.js | 179 ------------------ .../node-list/node-list-row.test.js | 167 ---------------- 2 files changed, 346 deletions(-) delete mode 100644 src/components/node-list/node-list-row.js delete mode 100644 src/components/node-list/node-list-row.test.js diff --git a/src/components/node-list/node-list-row.js b/src/components/node-list/node-list-row.js deleted file mode 100644 index 3b84d33d04..0000000000 --- a/src/components/node-list/node-list-row.js +++ /dev/null @@ -1,179 +0,0 @@ -import React, { memo } from 'react'; -import classnames from 'classnames'; -import { changed, replaceAngleBracketMatches } from '../../utils'; -import NodeIcon from '../icons/node-icon'; -import VisibleIcon from '../icons/visible'; -import InvisibleIcon from '../icons/invisible'; -import FocusModeIcon from '../icons/focus-mode'; -import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; - -// The exact fixed height of a row as measured by getBoundingClientRect() -export const nodeListRowHeight = 32; -/** - * Returns `true` if there are no props changes, therefore the last render can be reused. - * Performance: Checks only the minimal set of props known to change after first render. - */ -const shouldMemo = (prevProps, nextProps) => - !changed( - [ - 'active', - 'checked', - 'allUnchecked', - 'disabled', - 'faded', - 'focused', - 'visible', - 'selected', - 'highlight', - 'label', - 'children', - 'count', - ], - prevProps, - nextProps - ); - -export const NodeListRow = memo( - ({ - active, - allUnchecked, - checked, - children, - container: Container = 'div', - count, - disabled, - faded, - focused, - focusModeIcon = FocusModeIcon, - highlight, - icon, - id, - invisibleIcon = InvisibleIcon, - isSlicingPipelineApplied, - kind, - label, - name, - onChange, - onClick, - onMouseEnter, - onMouseLeave, - onToggleHoveredFocusMode, - rowType, - selected, - type, - visible, - visibleIcon = VisibleIcon, - }) => { - const isModularPipeline = type === 'modularPipeline'; - const FocusIcon = isModularPipeline ? focusModeIcon : null; - const isChecked = isModularPipeline ? checked || focused : checked; - const VisibilityIcon = isChecked ? visibleIcon : invisibleIcon; - const isButton = onClick && kind !== 'filter'; - const TextButton = isButton ? 'button' : 'div'; - - return ( - - {icon && ( - - )} - - - - {typeof count === 'number' && ( - - {count} - - )} - {VisibilityIcon && ( - - )} - {FocusIcon && ( - - )} - {children} - - ); - }, - shouldMemo -); diff --git a/src/components/node-list/node-list-row.test.js b/src/components/node-list/node-list-row.test.js deleted file mode 100644 index 95aa756021..0000000000 --- a/src/components/node-list/node-list-row.test.js +++ /dev/null @@ -1,167 +0,0 @@ -import React from 'react'; -import { NodeListRow, mapStateToProps } from './node-list-row'; -import { getNodeData } from '../../selectors/nodes'; -import { setup, mockState } from '../../utils/state.mock'; - -describe('NodeListRow', () => { - const node = getNodeData(mockState.spaceflights)[0]; - const setupProps = () => { - const props = { - active: true, - checked: true, - disabled: false, - faded: false, - visible: true, - id: node.id, - label: node.highlightedLabel, - name: node.name, - onClick: jest.fn(), - onMouseEnter: jest.fn(), - onMouseLeave: jest.fn(), - onChange: jest.fn(), - }; - return { props }; - }; - - it('renders without throwing', () => { - expect(() => setup.mount()).not.toThrow(); - }); - - describe('node list item', () => { - it('handles mouseenter events', () => { - const { props } = setupProps(); - const wrapper = setup.mount(); - const nodeRow = () => wrapper.find('.node-list-row'); - nodeRow().simulate('mouseenter'); - expect(props.onMouseEnter.mock.calls.length).toEqual(1); - }); - - it('handles mouseleave events', () => { - const { props } = setupProps(); - const wrapper = setup.mount(); - const nodeRow = () => wrapper.find('.node-list-row'); - nodeRow().simulate('mouseleave'); - expect(props.onMouseLeave.mock.calls.length).toEqual(1); - }); - - it('applies the overwrite class if not active', () => { - const { props } = setupProps(); - const activeNodeWrapper = setup.mount( - - ); - expect( - activeNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--overwrite') - ).toBe(true); - }); - - it('applies the overwrite class if not selected or active', () => { - const { props } = setupProps(); - const activeNodeWrapper = setup.mount( - - ); - expect( - activeNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--overwrite') - ).toBe(true); - }); - - it('does not applies the overwrite class if not selected', () => { - const { props } = setupProps(); - const activeNodeWrapper = setup.mount( - - ); - expect( - activeNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--overwrite') - ).toBe(false); - }); - - it('does not applies the overwrite class if active', () => { - const { props } = setupProps(); - const activeNodeWrapper = setup.mount( - - ); - expect( - activeNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--overwrite') - ).toBe(false); - }); - - it('uses active class if active', () => { - const { props } = setupProps(); - const activeNodeWrapper = setup.mount( - - ); - expect( - activeNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--active') - ).toBe(true); - }); - - it('uses disabled class if disabled (via type/tag only)', () => { - const { props } = setupProps(); - const disabledNodeWrapper = setup.mount( - - ); - expect( - disabledNodeWrapper - .find('.node-list-row') - .hasClass('node-list-row--disabled') - ).toBe(true); - }); - - it('shows count if count prop set', () => { - const { props } = setupProps(); - const mockCount = 123; - const wrapper = setup.mount(); - expect(wrapper.find('.node-list-row__count').text()).toBe( - mockCount.toString() - ); - }); - - it('does not show count if count prop not set', () => { - const { props } = setupProps(); - const wrapper = setup.mount(); - expect(wrapper.find('.node-list-row__count').exists()).toBe(false); - }); - - describe('focus mode', () => { - it('switches the visibility toggle from hide to show when the row is selected for focus mode', () => { - const { props } = setupProps(); - const wrapper = setup.mount( - - ); - expect(wrapper.find('VisibleIcon')).toHaveLength(1); - }); - }); - }); - - describe('node list item checkbox', () => { - const { props } = setupProps(); - const wrapper = setup.mount(); - const checkbox = () => wrapper.find('input'); - - it('handles toggle event', () => { - checkbox().simulate('change', { target: { checked: false } }); - expect(props.onChange.mock.calls.length).toEqual(1); - }); - }); - - it('maps state to props', () => { - const expectedResult = expect.objectContaining({ - active: expect.any(Boolean), - }); - expect(mapStateToProps(mockState.spaceflights, {})).toEqual(expectedResult); - }); -}); From a7ef9b650120bd2b2de2ed288b7cce9a54014281 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 16:06:11 +0200 Subject: [PATCH 16/46] separate the row-text component Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 37 ++++-------- src/components/filter-row/filter-row.scss | 47 --------------- .../node-list/node-list-row-list.js | 2 + src/components/row/row.js | 37 +++++------- src/components/row/row.scss | 55 ------------------ src/components/ui/row-text/row-text.js | 47 +++++++++++++++ src/components/ui/row-text/row-text.scss | 57 +++++++++++++++++++ 7 files changed, 129 insertions(+), 153 deletions(-) create mode 100644 src/components/ui/row-text/row-text.js create mode 100644 src/components/ui/row-text/row-text.scss diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index b25db37273..886fe84eb5 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -4,6 +4,7 @@ import { replaceAngleBracketMatches } from '../../utils'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; +import { RowText } from '../ui/row-text/row-text'; import './filter-row.scss'; @@ -13,7 +14,7 @@ export const FilterRow = ({ allUnchecked, checked, children, - container: Container = 'div', + dataTest, count, id, invisibleIcon = InvisibleIcon, @@ -22,44 +23,26 @@ export const FilterRow = ({ name, onChange, onClick, - onMouseEnter, - onMouseLeave, visible, visibleIcon = VisibleIcon, }) => { const VisibilityIcon = checked ? visibleIcon : invisibleIcon; return ( - - + name={children ? null : name} + label={label} + /> {count} @@ -74,6 +57,6 @@ export const FilterRow = ({ onChange={onChange} /> {children} - + ); }; diff --git a/src/components/filter-row/filter-row.scss b/src/components/filter-row/filter-row.scss index 0635de182c..b9b1be481b 100644 --- a/src/components/filter-row/filter-row.scss +++ b/src/components/filter-row/filter-row.scss @@ -22,53 +22,6 @@ } } -.filter-row__text { - display: flex; - align-items: center; - - // Fixed with required for overflow elipsis - width: calc(100% - 7em); - margin-right: auto; - padding: variables.$row-padding-y 0 variables.$row-padding-y 0; - color: inherit; - font-size: inherit; - font-family: inherit; - line-height: 1.6; - letter-spacing: inherit; - text-align: inherit; - background: none; - border: none; - border-radius: 0; - box-shadow: none; - cursor: default; - user-select: none; - - &--tree { - padding: variables.$row-padding-y 0 variables.$row-padding-y 1em; - } - - &:focus { - outline: none; - box-shadow: 0 0 0 4px var.$blue-300 inset; - - [data-whatintent='mouse'] & { - box-shadow: none; - } - } -} - -.filter-row__label { - overflow: hidden; - font-size: 1.4em; - white-space: nowrap; - text-overflow: ellipsis; - - b { - color: var(--color-nodelist-highlight); - font-weight: normal; - } -} - .filter-row__count { display: inline-block; flex-shrink: 0; diff --git a/src/components/node-list/node-list-row-list.js b/src/components/node-list/node-list-row-list.js index c65716da75..2642f26f03 100644 --- a/src/components/node-list/node-list-row-list.js +++ b/src/components/node-list/node-list-row-list.js @@ -2,6 +2,7 @@ import React from 'react'; import modifiers from '../../utils/modifiers'; import { FilterRow, nodeListRowHeight } from '../filter-row/filter-row'; import LazyList from '../lazy-list'; +import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; const NodeRowList = ({ items = [], @@ -53,6 +54,7 @@ const NodeRowList = ({ {items.slice(start, end).map((item) => ( - - - + onMouseEnter={onMouseEnter} + onMouseLeave={onMouseLeave} + rowType={rowType} + /> {VisibilityIcon && ( { + return ( + + ); +}; diff --git a/src/components/ui/row-text/row-text.scss b/src/components/ui/row-text/row-text.scss new file mode 100644 index 0000000000..0672254b84 --- /dev/null +++ b/src/components/ui/row-text/row-text.scss @@ -0,0 +1,57 @@ +@use '../../../styles/variables' as var; +@use '../../node-list/styles/variables'; + +.row-text { + display: flex; + align-items: center; + + // Fixed with required for overflow elipsis + width: calc(100% - 7em); + margin-right: auto; + padding: variables.$row-padding-y 0 variables.$row-padding-y 0; + color: inherit; + font-size: inherit; + font-family: inherit; + line-height: 1.6; + letter-spacing: inherit; + text-align: inherit; + background: none; + border: none; + border-radius: 0; + box-shadow: none; + cursor: default; + user-select: none; + + &--tree { + padding: variables.$row-padding-y 0 variables.$row-padding-y 1em; + } + + &:focus { + outline: none; + box-shadow: 0 0 0 4px var.$blue-300 inset; + + [data-whatintent='mouse'] & { + box-shadow: none; + } + } +} + +.row-text__label { + overflow: hidden; + font-size: 1.4em; + white-space: nowrap; + text-overflow: ellipsis; + + &--faded { + opacity: 0.65; + } + + &--disabled { + opacity: 0.3 !important; + } + + b { + color: var(--color-nodelist-highlight); + font-weight: normal; + } +} From 433d22295e8a8d0deab9eed3d0c11490ef7df8f2 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 17:44:47 +0200 Subject: [PATCH 17/46] include parent classname Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 17 ++++++--- .../node-list/node-list-row-list.js | 26 ++++--------- .../node-list/node-list-tree-item.js | 37 ++++++++++--------- src/components/row/row.js | 13 +++---- 4 files changed, 43 insertions(+), 50 deletions(-) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index 886fe84eb5..2c9554555a 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -1,6 +1,5 @@ import React from 'react'; import classnames from 'classnames'; -import { replaceAngleBracketMatches } from '../../utils'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; @@ -14,8 +13,8 @@ export const FilterRow = ({ allUnchecked, checked, children, - dataTest, count, + dataTest, id, invisibleIcon = InvisibleIcon, kind, @@ -23,6 +22,7 @@ export const FilterRow = ({ name, onChange, onClick, + parentClassName, visible, visibleIcon = VisibleIcon, }) => { @@ -30,10 +30,15 @@ export const FilterRow = ({ return (
{items.slice(start, end).map((item) => ( onItemChange(item, !e.target.checked)} + onClick={() => onItemClick(item)} + parentClassName={'node-list-filter-row'} visible={item.visible} - selected={item.selected} - highlight={item.highlight} - allUnchecked={group.allUnchecked} visibleIcon={item.visibleIcon} - invisibleIcon={item.invisibleIcon} - onClick={() => onItemClick(item)} - onMouseEnter={() => onItemMouseEnter(item)} - onMouseLeave={() => onItemMouseLeave(item)} - onChange={(e) => onItemChange(item, !e.target.checked)} - rowType="filter" /> ))} diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index 3491fe5c79..d27dfb5f02 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -3,6 +3,7 @@ import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; import { Row } from '../row/row'; +import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; const arrowIconColor = '#8e8e90'; @@ -25,35 +26,35 @@ const NodeListTreeItem = ({ expandIcon={} label={ onItemClick(data)} - onMouseEnter={() => onItemMouseEnter(data)} - onMouseLeave={() => onItemMouseLeave(data)} + isSlicingPipelineApplied={isSlicingPipelineApplied} + key={data.id} + kind="element" + label={data.highlightedLabel || data.name} + name={data.name} onChange={(e) => onItemChange(data, !e.target.checked, e.target.dataset.iconType) } + onClick={() => onItemClick(data)} + onMouseEnter={() => onItemMouseEnter(data)} + onMouseLeave={() => onItemMouseLeave(data)} onToggleHoveredFocusMode={onToggleHoveredFocusMode} + parentClassName={'node-list-tree-item-row'} rowType="tree" - focused={data.focused} + selected={data.selected} + type={data.type} + visible={data.visible} + visibleIcon={data.visibleIcon} /> } > diff --git a/src/components/row/row.js b/src/components/row/row.js index c31302f3ef..97d0a2c76c 100644 --- a/src/components/row/row.js +++ b/src/components/row/row.js @@ -15,10 +15,8 @@ export const nodeListRowHeight = 32; export const Row = ({ active, checked, - children, - container: Container = 'div', - disabled, dataTest, + disabled, faded, focused, focusModeIcon = FocusModeIcon, @@ -35,6 +33,7 @@ export const Row = ({ onMouseEnter, onMouseLeave, onToggleHoveredFocusMode, + parentClassName, rowType, selected, type, @@ -47,8 +46,8 @@ export const Row = ({ const VisibilityIcon = isChecked ? visibleIcon : invisibleIcon; return ( - )} - {children} - +
); }; From c51deaa2fe22b0d1a252df4a5cb11556e37b5a8b Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 20:13:32 +0200 Subject: [PATCH 18/46] update name for toggle-icon, to visibility-control Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 4 +- src/components/row/row.js | 6 +-- .../visibility-control.js} | 31 +++++++------ .../visibility-control.scss} | 36 +++++++-------- .../visibility-control.test.js} | 46 +++++++++---------- 5 files changed, 62 insertions(+), 61 deletions(-) rename src/components/ui/{toggle-icon/toggle-icon.js => visibility-control/visibility-control.js} (65%) rename src/components/ui/{toggle-icon/toggle-icon.scss => visibility-control/visibility-control.scss} (82%) rename src/components/ui/{toggle-icon/toggle-icon.test.js => visibility-control/visibility-control.test.js} (58%) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index 2c9554555a..5223614639 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -2,7 +2,7 @@ import React from 'react'; import classnames from 'classnames'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; -import { ToggleIcon } from '../ui/toggle-icon/toggle-icon'; +import { VisibilityControl } from '../ui/visibility-control/visibility-control'; import { RowText } from '../ui/row-text/row-text'; import './filter-row.scss'; @@ -51,7 +51,7 @@ export const FilterRow = ({ {count} - {VisibilityIcon && ( - )} {FocusIcon && ( - onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); + // update classname here const iconClassNames = classnames( className, - 'node-list-row-toggle--icon', - `node-list-row-toggle--icon--kind-${kind}`, + 'visibility-control--icon', + `visibility-control--icon--kind-${kind}`, { - 'node-list-row-toggle--icon--parent': isParent, - 'node-list-row-toggle--icon--child': !isParent, - 'node-list-row-toggle--icon--checked': isChecked, - 'node-list-row-toggle--icon--unchecked': !isChecked, - 'node-list-row-toggle--icon--all-unchecked': allUnchecked, - 'node-list-row-toggle--icon--focus-checked': focusChecked, + 'visibility-control--icon--parent': isParent, + 'visibility-control--icon--child': !isParent, + 'visibility-control--icon--checked': isChecked, + 'visibility-control--icon--unchecked': !isChecked, + 'visibility-control--icon--all-unchecked': allUnchecked, + 'visibility-control--icon--focus-checked': focusChecked, } ); const labelClassNames = classnames( - 'node-list-row-toggle', - `node-list-row-toggle--kind-${kind}`, + 'visibility-control', + `visibility-control--kind-${kind}`, { - 'node-list-row-toggle--disabled': disabled, - 'node-list-row-toggle--selected': selected, + 'visibility-control--disabled': disabled, + 'visibility-control--selected': selected, } ); const dataTestValue = getDataTestAttribute( - 'node-list-row-toggle', + 'visibility-control', kind === 'focus' ? 'focusMode' : 'visible', name ); diff --git a/src/components/ui/toggle-icon/toggle-icon.scss b/src/components/ui/visibility-control/visibility-control.scss similarity index 82% rename from src/components/ui/toggle-icon/toggle-icon.scss rename to src/components/ui/visibility-control/visibility-control.scss index 048f8f018f..fbbbaec5d5 100644 --- a/src/components/ui/toggle-icon/toggle-icon.scss +++ b/src/components/ui/visibility-control/visibility-control.scss @@ -2,7 +2,7 @@ @use '../../../styles/variables' as colors; @use '../../node-list/styles/variables'; -.node-list-row-toggle { +.visibility-control { cursor: pointer; &--kind-element { @@ -24,7 +24,7 @@ variables.$row-selected-dark ); -.node-list-row-toggle--selected::before { +.visibility-control--selected::before { opacity: 1; } @@ -34,7 +34,7 @@ // --- Toggle icon ---// -.node-list-row-toggle--icon { +.visibility-control--icon { width: variables.$toggle-icon-size; height: variables.$toggle-icon-size; padding: variables.$toggle-icon-padding; @@ -69,7 +69,7 @@ $element-icon-opacity-0: 0; $element-icon-opacity-1: 0.55; $element-icon-opacity-2: 1; -.node-list-row-toggle--icon--kind-element { +.visibility-control--icon--kind-element { // Change opacity on the SVG's child elements instead, in order to // maintain 100% opacity outline on parent SVG on keyboard focus > * { @@ -81,7 +81,7 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.node-list-row-toggle--icon--focus-checked { + &.visibility-control--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -99,14 +99,14 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.node-list-row-toggle--icon--checked { + &.visibility-control--icon--checked { > * { opacity: $element-icon-opacity-2; } } } - &.node-list-row-toggle--icon--focus-checked { + &.visibility-control--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -137,15 +137,15 @@ $filter-icon-opacity-1: 0.55; $filter-icon-opacity-2: 0.9; $filter-icon-opacity-3: 1; -.node-list-row-toggle--icon--kind-filter { +.visibility-control--icon--kind-filter { // Change opacity on the SVG's child elements instead, in order to // maintain 100% opacity outline on parent SVG on keyboard focus > * { opacity: $filter-icon-opacity-1; } - &.node-list-row-toggle--icon--all-unchecked, - .pipeline-nodelist__heading &.node-list-row-toggle--icon--all-unchecked > * { + &.visibility-control--icon--all-unchecked, + .pipeline-nodelist__heading &.visibility-control--icon--all-unchecked > * { > * { opacity: $filter-icon-opacity-0; } @@ -156,9 +156,9 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-1; } - &.node-list-row-toggle--icon--parent:hover, - &.node-list-row-toggle--icon--checked, - &.node-list-row-toggle--icon--child.node-list-row-toggle--icon--checked { + &.visibility-control--icon--parent:hover, + &.visibility-control--icon--checked, + &.visibility-control--icon--child.visibility-control--icon--checked { > * { opacity: $filter-icon-opacity-2; // Increase opacity for checked or parent hover } @@ -170,7 +170,7 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-2; // Increase opacity on keyboard focus } - &.node-list-row-toggle--icon--checked { + &.visibility-control--icon--checked { > * { opacity: $filter-icon-opacity-3; // Further increase for checked on focus } @@ -179,18 +179,18 @@ $filter-icon-opacity-3: 1; // --- Toggle (kind=filter) icon fills and strokes ---// - &.node-list-row-toggle--icon--checked { + &.visibility-control--icon--checked { fill: var(--color-nodelist-filter-indicator-on); stroke: var(--color-nodelist-filter-indicator-on); } - &.node-list-row-toggle--icon--unchecked { + &.visibility-control--icon--unchecked { fill: none; stroke: var(--color-nodelist-filter-indicator-off); } - .node-list-row:hover &.node-list-row-toggle--icon--all-unchecked, - &.node-list-row-toggle--icon--parent { + .node-list-row:hover &.visibility-control--icon--all-unchecked, + &.visibility-control--icon--parent { fill: colors.$blue-300; stroke: colors.$blue-300; } diff --git a/src/components/ui/toggle-icon/toggle-icon.test.js b/src/components/ui/visibility-control/visibility-control.test.js similarity index 58% rename from src/components/ui/toggle-icon/toggle-icon.test.js rename to src/components/ui/visibility-control/visibility-control.test.js index 59559976da..29b1c218d7 100644 --- a/src/components/ui/toggle-icon/toggle-icon.test.js +++ b/src/components/ui/visibility-control/visibility-control.test.js @@ -1,8 +1,8 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { ToggleIcon } from './node-list-row-toggle'; +import { VisibilityControl } from './visibility-control'; -describe('ToggleIcon', () => { +describe('VisibilityControl', () => { const baseProps = { name: 'Test Node', onChange: jest.fn(), @@ -11,75 +11,75 @@ describe('ToggleIcon', () => { it('applies "all-unchecked" class when allUnchecked is true', () => { const props = { ...baseProps, allUnchecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe( + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--all-unchecked')).toBe( true ); }); it('applies "disabled" class when disabled is true', () => { const props = { ...baseProps, disabled: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--disabled')).toBe(true); }); it('applies "checked" class when isChecked is true', () => { const props = { ...baseProps, isChecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--checked')).toBe(true); }); it('applies "parent" class when isParent is true', () => { const props = { ...baseProps, isParent: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--parent')).toBe(true); }); it('applies correct class for kind prop', () => { const kinds = ['modularPipeline', 'data', 'task']; kinds.forEach((kind) => { const props = { ...baseProps, kind }; - const wrapper = shallow(); - expect(wrapper.hasClass(`node-list-row-toggle--kind-${kind}`)).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass(`visibility-control--kind-${kind}`)).toBe(true); }); }); it('does not apply "all-unchecked" class when allUnchecked is false', () => { const props = { ...baseProps, allUnchecked: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--all-unchecked')).toBe( + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--all-unchecked')).toBe( false ); }); it('does not apply "disabled" class when disabled is false', () => { const props = { ...baseProps, disabled: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--disabled')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--disabled')).toBe(false); }); it('does not apply "checked" class when isChecked is false', () => { const props = { ...baseProps, isChecked: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--checked')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--checked')).toBe(false); }); it('does not apply "parent" class when isParent is false', () => { const props = { ...baseProps, isParent: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('node-list-row-toggle--icon--parent')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('visibility-control--icon--parent')).toBe(false); }); it('triggers onChange callback when clicked', () => { const props = { ...baseProps }; - const wrapper = shallow(); + const wrapper = shallow(); wrapper.simulate('click'); expect(props.onChange).toHaveBeenCalled(); }); it('does not trigger onToggleHoveredFocusMode when not provided', () => { const props = { ...baseProps, onToggleHoveredFocusMode: undefined }; - const wrapper = shallow(); + const wrapper = shallow(); wrapper.simulate('click'); // Since onToggleHoveredFocusMode is not provided, it should not throw an error expect(() => wrapper.simulate('click')).not.toThrow(); @@ -87,7 +87,7 @@ describe('ToggleIcon', () => { it('triggers onToggleHoveredFocusMode when provided and clicked', () => { const props = { ...baseProps }; - const wrapper = shallow(); + const wrapper = shallow(); wrapper.simulate('click'); expect(props.onToggleHoveredFocusMode).toHaveBeenCalled(); }); From 3774aec78d9fdf9f9251d9a14197b79df11fc600 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 17 Oct 2024 20:55:07 +0200 Subject: [PATCH 19/46] fix css and move nodeListRowHeight to config Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 2 -- src/components/node-list/node-list-row-list.js | 3 ++- src/components/row/row.js | 7 ++++--- .../ui/visibility-control/visibility-control.js | 2 +- .../visibility-control/visibility-control.scss | 16 ++++++---------- src/config.js | 3 +++ 6 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index 5223614639..71f0b0646d 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -7,8 +7,6 @@ import { RowText } from '../ui/row-text/row-text'; import './filter-row.scss'; -// The exact fixed height of a row as measured by getBoundingClientRect() -export const nodeListRowHeight = 32; export const FilterRow = ({ allUnchecked, checked, diff --git a/src/components/node-list/node-list-row-list.js b/src/components/node-list/node-list-row-list.js index 01faf1fc76..399e6b9abe 100644 --- a/src/components/node-list/node-list-row-list.js +++ b/src/components/node-list/node-list-row-list.js @@ -1,6 +1,7 @@ import React from 'react'; import modifiers from '../../utils/modifiers'; -import { FilterRow, nodeListRowHeight } from '../filter-row/filter-row'; +import { FilterRow } from '../filter-row/filter-row'; +import { nodeListRowHeight } from '../../config'; import LazyList from '../lazy-list'; import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; diff --git a/src/components/row/row.js b/src/components/row/row.js index 35ee03870f..f11bbec699 100644 --- a/src/components/row/row.js +++ b/src/components/row/row.js @@ -9,12 +9,10 @@ import { RowText } from '../ui/row-text/row-text'; import './row.scss'; -// The exact fixed height of a row as measured by getBoundingClientRect() -export const nodeListRowHeight = 32; - export const Row = ({ active, checked, + children, dataTest, disabled, faded, @@ -45,6 +43,8 @@ export const Row = ({ const isChecked = isModularPipeline ? checked || focused : checked; const VisibilityIcon = isChecked ? visibleIcon : invisibleIcon; + console.log(selected, 'selected'); + return (
* { opacity: $element-icon-opacity-1; } @@ -88,7 +86,7 @@ $element-icon-opacity-2: 1; } } - .node-list-row &:hover { + .node-list-tree-item-row &:hover { > * { opacity: $element-icon-opacity-2; } @@ -151,7 +149,7 @@ $filter-icon-opacity-3: 1; } } - .node-list-row:hover & { + .node-list-tree-item-row:hover & { > * { opacity: $filter-icon-opacity-1; } @@ -177,8 +175,6 @@ $filter-icon-opacity-3: 1; } } - // --- Toggle (kind=filter) icon fills and strokes ---// - &.visibility-control--icon--checked { fill: var(--color-nodelist-filter-indicator-on); stroke: var(--color-nodelist-filter-indicator-on); @@ -189,7 +185,7 @@ $filter-icon-opacity-3: 1; stroke: var(--color-nodelist-filter-indicator-off); } - .node-list-row:hover &.visibility-control--icon--all-unchecked, + .node-list-tree-item-row:hover &.visibility-control--icon--all-unchecked, &.visibility-control--icon--parent { fill: colors.$blue-300; stroke: colors.$blue-300; diff --git a/src/config.js b/src/config.js index 6bdaa8a1a2..daa602385a 100644 --- a/src/config.js +++ b/src/config.js @@ -35,6 +35,9 @@ export const codeSidebarWidth = { open: 480, }; +// The exact fixed height of a row as measured by getBoundingClientRect() +export const nodeListRowHeight = 32; + // These colours variables come from styles/variables const slate600 = '#0e222d'; const slate200 = '#21333e'; From 9c341ab139160e70d52a5b69af71691bae218839 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Fri, 18 Oct 2024 11:40:04 +0200 Subject: [PATCH 20/46] adding test for new component Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.test.js | 24 +++++++++ src/components/row/row.test.js | 54 +++++++++++++++++++ .../visibility-control/visibility-control.js | 1 - .../visibility-control.test.js | 36 ++----------- 4 files changed, 82 insertions(+), 33 deletions(-) create mode 100644 src/components/filter-row/filter-row.test.js create mode 100644 src/components/row/row.test.js diff --git a/src/components/filter-row/filter-row.test.js b/src/components/filter-row/filter-row.test.js new file mode 100644 index 0000000000..f9a004945e --- /dev/null +++ b/src/components/filter-row/filter-row.test.js @@ -0,0 +1,24 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import { FilterRow } from './filter-row'; + +describe('FilterRow Component', () => { + it('renders without crashing', () => { + const wrapper = mount(); + expect(wrapper.exists()).toBe(true); + }); + + it('renders correct visible classnames', () => { + const wrapper = mount(); + expect(wrapper.find('.filter-row').hasClass('filter-row--visible')).toBe( + true + ); + }); + + it('renders correct unchecked classnames', () => { + const wrapper = mount(); + expect(wrapper.find('.filter-row').hasClass('filter-row--unchecked')).toBe( + true + ); + }); +}); diff --git a/src/components/row/row.test.js b/src/components/row/row.test.js new file mode 100644 index 0000000000..4631fecdf7 --- /dev/null +++ b/src/components/row/row.test.js @@ -0,0 +1,54 @@ +import React from 'react'; +import { mount } from 'enzyme'; +import { Row } from './row'; + +// Mock props +const mockProps = { + name: 'Test Row', + kind: 'modular-pipeline', + active: false, + disabled: false, + selected: false, + visible: true, + onMouseEnter: jest.fn(), + onMouseLeave: jest.fn(), + onClick: jest.fn(), + icon: null, + type: 'modularPipeline', + checked: true, + focused: false, +}; + +describe('Row Component', () => { + it('renders without crashing', () => { + const wrapper = mount(); + expect(wrapper.find({ title: mockProps.name }).exists()).toBe(true); + }); + + it('handles mouse enter and leave events', () => { + const wrapper = mount(); + const nodeRow = () => wrapper.find('.row'); + + nodeRow().simulate('mouseenter'); + expect(mockProps.onMouseEnter).toHaveBeenCalled(); + nodeRow().simulate('mouseleave'); + expect(mockProps.onMouseLeave).toHaveBeenCalled(); + }); + + it('toggles visibility correctly', () => { + let wrapper = mount(); + const nodeRow = () => wrapper.find('.row'); + + expect(nodeRow().hasClass('row--visible')).toBe(true); + wrapper = mount(); + expect(nodeRow().hasClass('row--visible')).toBe(false); + }); + + it('updates class when the "selected" prop changes', () => { + let wrapper = mount(); + expect(wrapper.find('.row').hasClass('row--selected')).toBe(false); + wrapper.setProps({ selected: true }); + wrapper = wrapper.update(); + expect(wrapper.find('.row').hasClass('row--selected')).toBe(true); + }); +}); diff --git a/src/components/ui/visibility-control/visibility-control.js b/src/components/ui/visibility-control/visibility-control.js index 9f6bb8ffb0..826404edd4 100644 --- a/src/components/ui/visibility-control/visibility-control.js +++ b/src/components/ui/visibility-control/visibility-control.js @@ -23,7 +23,6 @@ export const VisibilityControl = ({ const handleMouseHover = (isEntering) => onToggleHoveredFocusMode && onToggleHoveredFocusMode(isEntering); - // update classname here const iconClassNames = classnames( className, 'visibility-control--icon', diff --git a/src/components/ui/visibility-control/visibility-control.test.js b/src/components/ui/visibility-control/visibility-control.test.js index 29b1c218d7..b32a11ee9d 100644 --- a/src/components/ui/visibility-control/visibility-control.test.js +++ b/src/components/ui/visibility-control/visibility-control.test.js @@ -9,32 +9,12 @@ describe('VisibilityControl', () => { onToggleHoveredFocusMode: jest.fn(), }; - it('applies "all-unchecked" class when allUnchecked is true', () => { - const props = { ...baseProps, allUnchecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--all-unchecked')).toBe( - true - ); - }); - it('applies "disabled" class when disabled is true', () => { const props = { ...baseProps, disabled: true }; const wrapper = shallow(); expect(wrapper.hasClass('visibility-control--disabled')).toBe(true); }); - it('applies "checked" class when isChecked is true', () => { - const props = { ...baseProps, isChecked: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--checked')).toBe(true); - }); - - it('applies "parent" class when isParent is true', () => { - const props = { ...baseProps, isParent: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--parent')).toBe(true); - }); - it('applies correct class for kind prop', () => { const kinds = ['modularPipeline', 'data', 'task']; kinds.forEach((kind) => { @@ -70,25 +50,17 @@ describe('VisibilityControl', () => { expect(wrapper.hasClass('visibility-control--icon--parent')).toBe(false); }); - it('triggers onChange callback when clicked', () => { - const props = { ...baseProps }; - const wrapper = shallow(); - wrapper.simulate('click'); - expect(props.onChange).toHaveBeenCalled(); - }); - it('does not trigger onToggleHoveredFocusMode when not provided', () => { const props = { ...baseProps, onToggleHoveredFocusMode: undefined }; const wrapper = shallow(); - wrapper.simulate('click'); - // Since onToggleHoveredFocusMode is not provided, it should not throw an error - expect(() => wrapper.simulate('click')).not.toThrow(); + wrapper.simulate('mouseenter'); + expect(() => wrapper.simulate('mouseenter')).not.toThrow(); }); - it('triggers onToggleHoveredFocusMode when provided and clicked', () => { + it('triggers onToggleHoveredFocusMode when provided', () => { const props = { ...baseProps }; const wrapper = shallow(); - wrapper.simulate('click'); + wrapper.simulate('mouseenter'); expect(props.onToggleHoveredFocusMode).toHaveBeenCalled(); }); }); From 46830b8d53b774ad54c5ee5cdc130003c6926403 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Fri, 18 Oct 2024 13:06:17 +0200 Subject: [PATCH 21/46] update classname for tests Signed-off-by: Huong Nguyen --- .../node-list/node-list-row-list.js | 2 - src/components/node-list/node-list.test.js | 38 ++++++++++--------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/components/node-list/node-list-row-list.js b/src/components/node-list/node-list-row-list.js index 399e6b9abe..c6795624d6 100644 --- a/src/components/node-list/node-list-row-list.js +++ b/src/components/node-list/node-list-row-list.js @@ -11,8 +11,6 @@ const NodeRowList = ({ collapsed, onItemClick, onItemChange, - onItemMouseEnter, - onItemMouseLeave, }) => ( (end - start) * nodeListRowHeight} diff --git a/src/components/node-list/node-list.test.js b/src/components/node-list/node-list.test.js index de1f8e20c2..4265a7ef48 100644 --- a/src/components/node-list/node-list.test.js +++ b/src/components/node-list/node-list.test.js @@ -59,7 +59,7 @@ describe('NodeList', () => { const search = () => wrapper.find('.search-input__field'); search().simulate('change', { target: { value: searchText } }); const nodeList = wrapper.find( - '.pipeline-nodelist__elements-panel .node-list-row' + '.pipeline-nodelist__elements-panel .node-list-tree-item-row' ); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -101,7 +101,9 @@ describe('NodeList', () => { // Re-find elements from root each time to see updates const search = () => wrapper.find('.search-input__field'); const nodeList = () => - wrapper.find('.pipeline-nodelist__elements-panel .node-list-row'); + wrapper.find( + '.pipeline-nodelist__elements-panel .node-list-tree-item-row' + ); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -146,7 +148,9 @@ describe('NodeList', () => { // Re-find elements from root each time to see updates const search = () => wrapper.find('.search-input__field'); const nodeList = () => - wrapper.find('.pipeline-nodelist__elements-panel .node-list-row'); + wrapper.find( + '.pipeline-nodelist__elements-panel .node-list-tree-item-row' + ); const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -188,7 +192,7 @@ describe('NodeList', () => { const elements = (wrapper) => wrapper .find('.MuiTreeItem-label') - .find('.node-list-row') + .find('.node-list-tree-item-row') .map((row) => [row.prop('title')]); it('shows full node names when pretty name is turned off', () => { @@ -229,10 +233,10 @@ describe('NodeList', () => { describe('checkboxes on tag filter items', () => { const checkboxByName = (wrapper, text) => - wrapper.find(`.node-list-row__checkbox[name="${text}"]`); + wrapper.find(`.visibility-control__checkbox[name="${text}"]`); const rowByName = (wrapper, text) => - wrapper.find(`.node-list-row[title="${text}"]`); + wrapper.find(`.node-list-tree-item-row[title="${text}"]`); const changeRows = (wrapper, names, checked) => names.forEach((name) => @@ -244,11 +248,8 @@ describe('NodeList', () => { const elements = (wrapper) => wrapper .find('.MuiTreeItem-label') - .find('.node-list-row') - .map((row) => [ - row.prop('title'), - !row.hasClass('node-list-row--disabled'), - ]); + .find('.node-list-tree-item-row') + .map((row) => [row.prop('title'), !row.hasClass('row--disabled')]); const elementsEnabled = (wrapper) => { return elements(wrapper).filter(([_, enabled]) => enabled); @@ -327,7 +328,7 @@ describe('NodeList', () => { ); - const uncheckedClass = 'node-list-row--unchecked'; + const uncheckedClass = 'visibility-control--icon--unchecked'; expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( true @@ -379,6 +380,7 @@ describe('NodeList', () => { }); }); + // FILTER GROUP describe('node list', () => { it('renders the correct number of tags in the filter panel', () => { const wrapper = setup.mount( @@ -387,7 +389,7 @@ describe('NodeList', () => { ); const nodeList = wrapper.find( - '.pipeline-nodelist__list--nested .node-list-row' + '.pipeline-nodelist__list--nested .node-list-filter-row' ); // const nodes = getNodeData(mockState.spaceflights); const tags = getTagData(mockState.spaceflights); @@ -440,16 +442,16 @@ describe('NodeList', () => { ); // this needs to be the 3rd element as the first 2 elements are modular pipelines rows which does not apply the '--active' class - const nodeRow = () => wrapper.find('.node-list-row').at(3); + const nodeRow = () => wrapper.find('.node-list-tree-item-row').at(3); it('handles mouseenter events', () => { nodeRow().simulate('mouseenter'); - expect(nodeRow().hasClass('node-list-row--active')).toBe(true); + expect(nodeRow().hasClass('.row--active')).toBe(true); }); it('handles mouseleave events', () => { nodeRow().simulate('mouseleave'); - expect(nodeRow().hasClass('node-list-row--active')).toBe(false); + expect(nodeRow().hasClass('.row--active')).toBe(false); }); }); @@ -459,7 +461,7 @@ describe('NodeList', () => { ); - const checkbox = () => wrapper.find('.node-list-row input').at(4); + const checkbox = () => wrapper.find('.node-list-tree-item-row input').at(4); it('handles toggle off event', () => { checkbox().simulate('change', { @@ -503,7 +505,7 @@ describe('NodeList', () => { it('After applying any filter filter button should not be disabled', () => { const nodeTypeFilter = wrapper.find( - `.node-list-row__checkbox[name="Datasets"]` + `.visibility-control__checkbox[name="Datasets"]` ); nodeTypeFilter.simulate('click'); From c9bd1bb1260445bfeee4acf86af61eca6e5427cf Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 22 Oct 2024 12:13:09 +0100 Subject: [PATCH 22/46] move row inside node-list Signed-off-by: Huong Nguyen --- src/components/{ => node-list/components}/row/row.js | 12 ++++++------ .../{ => node-list/components}/row/row.scss | 4 ++-- .../{ => node-list/components}/row/row.test.js | 0 src/components/node-list/node-list-tree-item.js | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) rename src/components/{ => node-list/components}/row/row.js (89%) rename src/components/{ => node-list/components}/row/row.scss (96%) rename src/components/{ => node-list/components}/row/row.test.js (100%) diff --git a/src/components/row/row.js b/src/components/node-list/components/row/row.js similarity index 89% rename from src/components/row/row.js rename to src/components/node-list/components/row/row.js index f11bbec699..e23c17294d 100644 --- a/src/components/row/row.js +++ b/src/components/node-list/components/row/row.js @@ -1,11 +1,11 @@ import React from 'react'; import classnames from 'classnames'; -import NodeIcon from '../icons/node-icon'; -import VisibleIcon from '../icons/visible'; -import InvisibleIcon from '../icons/invisible'; -import FocusModeIcon from '../icons/focus-mode'; -import { VisibilityControl } from '../ui/visibility-control/visibility-control'; -import { RowText } from '../ui/row-text/row-text'; +import NodeIcon from '../../../icons/node-icon'; +import VisibleIcon from '../../../icons/visible'; +import InvisibleIcon from '../../../icons/invisible'; +import FocusModeIcon from '../../../icons/focus-mode'; +import { VisibilityControl } from '../../../ui/visibility-control/visibility-control'; +import { RowText } from '../../../ui/row-text/row-text'; import './row.scss'; diff --git a/src/components/row/row.scss b/src/components/node-list/components/row/row.scss similarity index 96% rename from src/components/row/row.scss rename to src/components/node-list/components/row/row.scss index 799f788188..a492bf8458 100644 --- a/src/components/row/row.scss +++ b/src/components/node-list/components/row/row.scss @@ -1,5 +1,5 @@ -@use '../../styles/variables' as var; -@use '../node-list/styles/variables'; +@use '../../../../styles/variables' as var; +@use '../../styles/variables'; .MuiTreeItem-iconContainer svg { z-index: var.$zindex-MuiTreeItem-icon; diff --git a/src/components/row/row.test.js b/src/components/node-list/components/row/row.test.js similarity index 100% rename from src/components/row/row.test.js rename to src/components/node-list/components/row/row.test.js diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index d27dfb5f02..94d39ceb81 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -2,7 +2,7 @@ import React from 'react'; import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; -import { Row } from '../row/row'; +import { Row } from './components/row/row'; import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; const arrowIconColor = '#8e8e90'; From 80428af0f5488755a820784bd6830362a79860dd Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 22 Oct 2024 14:19:33 +0100 Subject: [PATCH 23/46] connect redux store to component Signed-off-by: Huong Nguyen --- .../node-list/components/row/row.js | 29 +++++++++++++------ .../node-list/node-list-tree-item.js | 2 +- .../visibility-control/visibility-control.js | 0 3 files changed, 21 insertions(+), 10 deletions(-) mode change 100644 => 100755 src/components/ui/visibility-control/visibility-control.js diff --git a/src/components/node-list/components/row/row.js b/src/components/node-list/components/row/row.js index e23c17294d..c24623b9cc 100644 --- a/src/components/node-list/components/row/row.js +++ b/src/components/node-list/components/row/row.js @@ -1,4 +1,6 @@ import React from 'react'; +import { connect } from 'react-redux'; +import { uniqueId } from 'lodash'; import classnames from 'classnames'; import NodeIcon from '../../../icons/node-icon'; import VisibleIcon from '../../../icons/visible'; @@ -6,10 +8,11 @@ import InvisibleIcon from '../../../icons/invisible'; import FocusModeIcon from '../../../icons/focus-mode'; import { VisibilityControl } from '../../../ui/visibility-control/visibility-control'; import { RowText } from '../../../ui/row-text/row-text'; +import { getNodeActive } from '../../../../selectors/nodes'; import './row.scss'; -export const Row = ({ +const Row = ({ active, checked, children, @@ -43,10 +46,8 @@ export const Row = ({ const isChecked = isModularPipeline ? checked || focused : checked; const VisibilityIcon = isChecked ? visibleIcon : invisibleIcon; - console.log(selected, 'selected'); - return ( -
)} -
+ ); }; + +export const mapStateToProps = (state, ownProps) => ({ + ...ownProps, + active: + typeof ownProps.active !== 'undefined' + ? ownProps.active + : getNodeActive(state)[ownProps.id] || false, +}); + +export default connect(mapStateToProps)(Row); diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index 94d39ceb81..c3e8337b5e 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -2,7 +2,7 @@ import React from 'react'; import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; -import { Row } from './components/row/row'; +import Row from './components/row/row'; import { getDataTestAttribute } from '../../utils/get-data-test-attribute'; const arrowIconColor = '#8e8e90'; diff --git a/src/components/ui/visibility-control/visibility-control.js b/src/components/ui/visibility-control/visibility-control.js old mode 100644 new mode 100755 From 55f6e3f15f95d979f02f84c161104cadb12b1da4 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 22 Oct 2024 15:40:48 +0100 Subject: [PATCH 24/46] fix styling Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 5 +++-- src/components/filter-row/filter-row.scss | 6 +++--- src/components/node-list/styles/_group.scss | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index 71f0b0646d..833bae9de9 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -2,6 +2,7 @@ import React from 'react'; import classnames from 'classnames'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; +import IndicatorIcon from '../icons/indicator'; import { VisibilityControl } from '../ui/visibility-control/visibility-control'; import { RowText } from '../ui/row-text/row-text'; @@ -24,7 +25,7 @@ export const FilterRow = ({ visible, visibleIcon = VisibleIcon, }) => { - const VisibilityIcon = checked ? visibleIcon : invisibleIcon; + const Icon = checked ? visibleIcon : invisibleIcon; return (
Date: Tue, 22 Oct 2024 17:26:40 +0100 Subject: [PATCH 25/46] update name to ToggleControl Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 5 +-- .../node-list/components/row/row.js | 6 +-- src/components/node-list/node-list.test.js | 6 +-- .../toggle-control.js} | 36 ++++++++--------- .../toggle-control.scss} | 40 +++++++++---------- .../toggle-control.test.js} | 34 ++++++++-------- 6 files changed, 62 insertions(+), 65 deletions(-) mode change 100644 => 100755 src/components/node-list/components/row/row.js rename src/components/ui/{visibility-control/visibility-control.js => toggle-control/toggle-control.js} (62%) rename src/components/ui/{visibility-control/visibility-control.scss => toggle-control/toggle-control.scss} (80%) rename src/components/ui/{visibility-control/visibility-control.test.js => toggle-control/toggle-control.test.js} (58%) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index 833bae9de9..a08191eabc 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -2,8 +2,7 @@ import React from 'react'; import classnames from 'classnames'; import VisibleIcon from '../icons/visible'; import InvisibleIcon from '../icons/invisible'; -import IndicatorIcon from '../icons/indicator'; -import { VisibilityControl } from '../ui/visibility-control/visibility-control'; +import { ToggleControl } from '../ui/toggle-control/toggle-control'; import { RowText } from '../ui/row-text/row-text'; import './filter-row.scss'; @@ -50,7 +49,7 @@ export const FilterRow = ({ {count} - {VisibilityIcon && ( - )} {FocusIcon && ( - { describe('checkboxes on tag filter items', () => { const checkboxByName = (wrapper, text) => - wrapper.find(`.visibility-control__checkbox[name="${text}"]`); + wrapper.find(`.toggle-control__checkbox[name="${text}"]`); const rowByName = (wrapper, text) => wrapper.find(`.node-list-tree-item-row[title="${text}"]`); @@ -328,7 +328,7 @@ describe('NodeList', () => { ); - const uncheckedClass = 'visibility-control--icon--unchecked'; + const uncheckedClass = 'toggle-control--icon--unchecked'; expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( true @@ -505,7 +505,7 @@ describe('NodeList', () => { it('After applying any filter filter button should not be disabled', () => { const nodeTypeFilter = wrapper.find( - `.visibility-control__checkbox[name="Datasets"]` + `.toggle-control__checkbox[name="Datasets"]` ); nodeTypeFilter.simulate('click'); diff --git a/src/components/ui/visibility-control/visibility-control.js b/src/components/ui/toggle-control/toggle-control.js similarity index 62% rename from src/components/ui/visibility-control/visibility-control.js rename to src/components/ui/toggle-control/toggle-control.js index 826404edd4..5544442157 100755 --- a/src/components/ui/visibility-control/visibility-control.js +++ b/src/components/ui/toggle-control/toggle-control.js @@ -2,17 +2,17 @@ import React from 'react'; import classnames from 'classnames'; import { getDataTestAttribute } from '../../../utils/get-data-test-attribute'; -import './visibility-control.scss'; +import './toggle-control.scss'; -export const VisibilityControl = ({ - allUnchecked, +export const ToggleControl = ({ + // allUnchecked, className, disabled, focusChecked, IconComponent, id, isChecked, - isParent, + // children, kind, name, onChange, @@ -25,29 +25,29 @@ export const VisibilityControl = ({ const iconClassNames = classnames( className, - 'visibility-control--icon', - `visibility-control--icon--kind-${kind}`, + 'toggle-control--icon', + `toggle-control--icon--kind-${kind}`, { - 'visibility-control--icon--parent': isParent, - 'visibility-control--icon--child': !isParent, - 'visibility-control--icon--checked': isChecked, - 'visibility-control--icon--unchecked': !isChecked, - 'visibility-control--icon--all-unchecked': allUnchecked, - 'visibility-control--icon--focus-checked': focusChecked, + // 'toggle-control--icon--parent': Boolean(children), + // 'toggle-control--icon--child': Boolean(children), + 'toggle-control--icon--checked': isChecked, + 'toggle-control--icon--unchecked': !isChecked, + // 'toggle-control--icon--all-unchecked': allUnchecked, + 'toggle-control--icon--focus-checked': focusChecked, } ); const labelClassNames = classnames( - 'visibility-control', - `visibility-control--kind-${kind}`, + 'toggle-control', + `toggle-control--kind-${kind}`, { - 'visibility-control--disabled': disabled, - 'visibility-control--selected': selected, + 'toggle-control--disabled': disabled, + 'toggle-control--selected': selected, } ); const dataTestValue = getDataTestAttribute( - 'visibility-control', + 'toggle-control', kind === 'focus' ? 'focusMode' : 'visible', name ); @@ -62,7 +62,7 @@ export const VisibilityControl = ({ > * { @@ -79,7 +79,7 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.visibility-control--icon--focus-checked { + &.toggle-control--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -97,14 +97,14 @@ $element-icon-opacity-2: 1; opacity: $element-icon-opacity-1; } - &.visibility-control--icon--checked { + &.toggle-control--icon--checked { > * { opacity: $element-icon-opacity-2; } } } - &.visibility-control--icon--focus-checked { + &.toggle-control--icon--focus-checked { > * { opacity: $element-icon-opacity-2; } @@ -135,15 +135,15 @@ $filter-icon-opacity-1: 0.55; $filter-icon-opacity-2: 0.9; $filter-icon-opacity-3: 1; -.visibility-control--icon--kind-filter { +.toggle-control--icon--kind-filter { // Change opacity on the SVG's child elements instead, in order to // maintain 100% opacity outline on parent SVG on keyboard focus > * { opacity: $filter-icon-opacity-1; } - &.visibility-control--icon--all-unchecked, - .pipeline-nodelist__heading &.visibility-control--icon--all-unchecked > * { + &.toggle-control--icon--all-unchecked, + .pipeline-nodelist__heading &.toggle-control--icon--all-unchecked > * { > * { opacity: $filter-icon-opacity-0; } @@ -154,9 +154,9 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-1; } - &.visibility-control--icon--parent:hover, - &.visibility-control--icon--checked, - &.visibility-control--icon--child.visibility-control--icon--checked { + &.toggle-control--icon--parent:hover, + &.toggle-control--icon--checked, + &.toggle-control--icon--child.toggle-control--icon--checked { > * { opacity: $filter-icon-opacity-2; // Increase opacity for checked or parent hover } @@ -168,25 +168,25 @@ $filter-icon-opacity-3: 1; opacity: $filter-icon-opacity-2; // Increase opacity on keyboard focus } - &.visibility-control--icon--checked { + &.toggle-control--icon--checked { > * { opacity: $filter-icon-opacity-3; // Further increase for checked on focus } } } - &.visibility-control--icon--checked { + &.toggle-control--icon--checked { fill: var(--color-nodelist-filter-indicator-on); stroke: var(--color-nodelist-filter-indicator-on); } - &.visibility-control--icon--unchecked { + &.toggle-control--icon--unchecked { fill: none; stroke: var(--color-nodelist-filter-indicator-off); } - .node-list-tree-item-row:hover &.visibility-control--icon--all-unchecked, - &.visibility-control--icon--parent { + .node-list-tree-item-row:hover &.toggle-control--icon--all-unchecked, + &.toggle-control--icon--parent { fill: colors.$blue-300; stroke: colors.$blue-300; } diff --git a/src/components/ui/visibility-control/visibility-control.test.js b/src/components/ui/toggle-control/toggle-control.test.js similarity index 58% rename from src/components/ui/visibility-control/visibility-control.test.js rename to src/components/ui/toggle-control/toggle-control.test.js index b32a11ee9d..088a3f2d48 100644 --- a/src/components/ui/visibility-control/visibility-control.test.js +++ b/src/components/ui/toggle-control/toggle-control.test.js @@ -1,8 +1,8 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { VisibilityControl } from './visibility-control'; +import { ToggleControl } from './toggle-control'; -describe('VisibilityControl', () => { +describe('ToggleControl', () => { const baseProps = { name: 'Test Node', onChange: jest.fn(), @@ -11,55 +11,53 @@ describe('VisibilityControl', () => { it('applies "disabled" class when disabled is true', () => { const props = { ...baseProps, disabled: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--disabled')).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass('toggle-control--disabled')).toBe(true); }); it('applies correct class for kind prop', () => { const kinds = ['modularPipeline', 'data', 'task']; kinds.forEach((kind) => { const props = { ...baseProps, kind }; - const wrapper = shallow(); - expect(wrapper.hasClass(`visibility-control--kind-${kind}`)).toBe(true); + const wrapper = shallow(); + expect(wrapper.hasClass(`toggle-control--kind-${kind}`)).toBe(true); }); }); it('does not apply "all-unchecked" class when allUnchecked is false', () => { const props = { ...baseProps, allUnchecked: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--all-unchecked')).toBe( - false - ); + const wrapper = shallow(); + expect(wrapper.hasClass('toggle-control--icon--all-unchecked')).toBe(false); }); it('does not apply "disabled" class when disabled is false', () => { const props = { ...baseProps, disabled: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--disabled')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('toggle-control--disabled')).toBe(false); }); it('does not apply "checked" class when isChecked is false', () => { const props = { ...baseProps, isChecked: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--checked')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('toggle-control--icon--checked')).toBe(false); }); it('does not apply "parent" class when isParent is false', () => { const props = { ...baseProps, isParent: false }; - const wrapper = shallow(); - expect(wrapper.hasClass('visibility-control--icon--parent')).toBe(false); + const wrapper = shallow(); + expect(wrapper.hasClass('toggle-control--icon--parent')).toBe(false); }); it('does not trigger onToggleHoveredFocusMode when not provided', () => { const props = { ...baseProps, onToggleHoveredFocusMode: undefined }; - const wrapper = shallow(); + const wrapper = shallow(); wrapper.simulate('mouseenter'); expect(() => wrapper.simulate('mouseenter')).not.toThrow(); }); it('triggers onToggleHoveredFocusMode when provided', () => { const props = { ...baseProps }; - const wrapper = shallow(); + const wrapper = shallow(); wrapper.simulate('mouseenter'); expect(props.onToggleHoveredFocusMode).toHaveBeenCalled(); }); From b9fb79b1ef74953a0616476c08832de9473f3d82 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Tue, 22 Oct 2024 23:00:36 +0100 Subject: [PATCH 26/46] remove disable props as no longer needed Signed-off-by: Huong Nguyen --- src/components/node-list/node-list-group.js | 2 -- src/components/node-list/node-list-group.test.js | 12 ------------ src/components/ui/toggle-control/toggle-control.js | 8 -------- .../ui/toggle-control/toggle-control.test.js | 6 ------ 4 files changed, 28 deletions(-) diff --git a/src/components/node-list/node-list-group.js b/src/components/node-list/node-list-group.js index a90becae78..0de844e547 100644 --- a/src/components/node-list/node-list-group.js +++ b/src/components/node-list/node-list-group.js @@ -38,7 +38,6 @@ export const NodeListGroup = ({ onToggleCollapsed(id)} diff --git a/src/components/node-list/node-list-group.test.js b/src/components/node-list/node-list-group.test.js index e8ca53fb8a..940f9bedc0 100644 --- a/src/components/node-list/node-list-group.test.js +++ b/src/components/node-list/node-list-group.test.js @@ -73,16 +73,4 @@ describe('NodeListGroup', () => { false ); }); - - it('adds disabled class when items list is empty', () => { - const type = getNodeTypes(mockState.spaceflights)[0]; - const wrapper = setup.mount( - - ); - 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/ui/toggle-control/toggle-control.js b/src/components/ui/toggle-control/toggle-control.js index 5544442157..33dfdb4122 100755 --- a/src/components/ui/toggle-control/toggle-control.js +++ b/src/components/ui/toggle-control/toggle-control.js @@ -5,14 +5,11 @@ import { getDataTestAttribute } from '../../../utils/get-data-test-attribute'; import './toggle-control.scss'; export const ToggleControl = ({ - // allUnchecked, className, - disabled, focusChecked, IconComponent, id, isChecked, - // children, kind, name, onChange, @@ -28,11 +25,8 @@ export const ToggleControl = ({ 'toggle-control--icon', `toggle-control--icon--kind-${kind}`, { - // 'toggle-control--icon--parent': Boolean(children), - // 'toggle-control--icon--child': Boolean(children), 'toggle-control--icon--checked': isChecked, 'toggle-control--icon--unchecked': !isChecked, - // 'toggle-control--icon--all-unchecked': allUnchecked, 'toggle-control--icon--focus-checked': focusChecked, } ); @@ -41,7 +35,6 @@ export const ToggleControl = ({ 'toggle-control', `toggle-control--kind-${kind}`, { - 'toggle-control--disabled': disabled, 'toggle-control--selected': selected, } ); @@ -66,7 +59,6 @@ export const ToggleControl = ({ data-test={dataTestValue} type="checkbox" checked={isChecked} - disabled={disabled} name={name} onChange={onChange} data-icon-type={dataIconType} diff --git a/src/components/ui/toggle-control/toggle-control.test.js b/src/components/ui/toggle-control/toggle-control.test.js index 088a3f2d48..e99cccff0b 100644 --- a/src/components/ui/toggle-control/toggle-control.test.js +++ b/src/components/ui/toggle-control/toggle-control.test.js @@ -9,12 +9,6 @@ describe('ToggleControl', () => { onToggleHoveredFocusMode: jest.fn(), }; - it('applies "disabled" class when disabled is true', () => { - const props = { ...baseProps, disabled: true }; - const wrapper = shallow(); - expect(wrapper.hasClass('toggle-control--disabled')).toBe(true); - }); - it('applies correct class for kind prop', () => { const kinds = ['modularPipeline', 'data', 'task']; kinds.forEach((kind) => { From 01f24985b21cdc748356e2141c57d73eb8321b61 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 23 Oct 2024 15:09:18 +0100 Subject: [PATCH 27/46] replace js code with css to simplify the code Signed-off-by: Huong Nguyen --- .../node-list/components/row/row.js | 13 +--- .../node-list/components/row/row.scss | 5 -- .../node-list/components/row/row.test.js | 57 +++++++++----- src/components/node-list/node-list.test.js | 76 ++++++------------- .../node-list/styles/node-list.scss | 28 ++++++- 5 files changed, 87 insertions(+), 92 deletions(-) diff --git a/src/components/node-list/components/row/row.js b/src/components/node-list/components/row/row.js index 821f633e10..99086ac2a2 100755 --- a/src/components/node-list/components/row/row.js +++ b/src/components/node-list/components/row/row.js @@ -38,7 +38,6 @@ const Row = ({ rowType, selected, type, - visible, visibleIcon = VisibleIcon, }) => { const isModularPipeline = type === 'modularPipeline'; @@ -49,11 +48,9 @@ const Row = ({ return (
({ - ...ownProps, - active: - typeof ownProps.active !== 'undefined' - ? ownProps.active - : getNodeActive(state)[ownProps.id] || false, -}); - -export default connect(mapStateToProps)(Row); +export default Row; diff --git a/src/components/node-list/components/row/row.scss b/src/components/node-list/components/row/row.scss index a492bf8458..2e688c9156 100644 --- a/src/components/node-list/components/row/row.scss +++ b/src/components/node-list/components/row/row.scss @@ -26,11 +26,6 @@ } } - &--active, - &--visible:hover { - background-color: var(--color-nodelist-row-active); - } - &--selected, &--visible#{&}--selected { // Additional selector required to increase specificity to override previous rule diff --git a/src/components/node-list/components/row/row.test.js b/src/components/node-list/components/row/row.test.js index 4631fecdf7..f26723f006 100644 --- a/src/components/node-list/components/row/row.test.js +++ b/src/components/node-list/components/row/row.test.js @@ -1,6 +1,6 @@ import React from 'react'; -import { mount } from 'enzyme'; -import { Row } from './row'; +import Row from './row'; +import { setup } from '../../../../utils/state.mock'; // Mock props const mockProps = { @@ -21,34 +21,51 @@ const mockProps = { describe('Row Component', () => { it('renders without crashing', () => { - const wrapper = mount(); - expect(wrapper.find({ title: mockProps.name }).exists()).toBe(true); + expect(() => setup.mount()).not.toThrow(); }); - it('handles mouse enter and leave events', () => { - const wrapper = mount(); + it('handles mouseenter events', () => { + const wrapper = setup.mount(); const nodeRow = () => wrapper.find('.row'); - nodeRow().simulate('mouseenter'); - expect(mockProps.onMouseEnter).toHaveBeenCalled(); - nodeRow().simulate('mouseleave'); - expect(mockProps.onMouseLeave).toHaveBeenCalled(); + expect(mockProps.onMouseEnter.mock.calls.length).toEqual(1); }); - it('toggles visibility correctly', () => { - let wrapper = mount(); + it('handles mouseleave events', () => { + const wrapper = setup.mount(); const nodeRow = () => wrapper.find('.row'); + nodeRow().simulate('mouseleave'); + expect(mockProps.onMouseLeave.mock.calls.length).toEqual(1); + }); - expect(nodeRow().hasClass('row--visible')).toBe(true); - wrapper = mount(); - expect(nodeRow().hasClass('row--visible')).toBe(false); + it('applies the row--active class when active is true', () => { + const wrapper = setup.mount(); + expect(wrapper.find('.row').hasClass('row--active')).toBe(true); }); - it('updates class when the "selected" prop changes', () => { - let wrapper = mount(); - expect(wrapper.find('.row').hasClass('row--selected')).toBe(false); - wrapper.setProps({ selected: true }); - wrapper = wrapper.update(); + it('applies the row--selected class when selected is true', () => { + const wrapper = setup.mount(); expect(wrapper.find('.row').hasClass('row--selected')).toBe(true); }); + + it('applies the row--selected class when highlight is true and isSlicingPipelineApplied is false', () => { + const wrapper = setup.mount( + + ); + expect(wrapper.find('.row').hasClass('row--selected')).toBe(true); + }); + + it('applies the row--disabled class when disabled is true', () => { + const wrapper = setup.mount(); + expect(wrapper.find('.row').hasClass('row--disabled')).toBe(true); + }); + + it('applies the overwrite class if not selected or active', () => { + const activeNodeWrapper = setup.mount( + + ); + expect(activeNodeWrapper.find('.row').hasClass('row--overwrite')).toBe( + true + ); + }); }); diff --git a/src/components/node-list/node-list.test.js b/src/components/node-list/node-list.test.js index 659e6d7292..d6c3b39318 100644 --- a/src/components/node-list/node-list.test.js +++ b/src/components/node-list/node-list.test.js @@ -322,26 +322,28 @@ describe('NodeList', () => { expect(tagItem(wrapper).hasClass(uncheckedClass)).toBe(true); }); - it('adds a class to the row when a tag row unchecked', () => { - const wrapper = setup.mount( - - - - ); - const uncheckedClass = 'toggle-control--icon--unchecked'; - - expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( - true - ); - changeRows(wrapper, ['Preprocessing'], true); - expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( - false - ); - changeRows(wrapper, ['Preprocessing'], false); - expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( - true - ); - }); + // it('adds a class to the row when a tag row unchecked', () => { + // const wrapper = setup.mount( + // + // + // + // ); + + // console.log(wrapper.debug(), 'NodeList debug'); + // const uncheckedClass = 'toggle-control--icon--unchecked'; + + // eexpect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( + // true + // ); + // changeRows(wrapper, ['Preprocessing'], true); + // expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( + // false + // ); + // changeRows(wrapper, ['Preprocessing'], false); + // expect(rowByName(wrapper, 'Preprocessing').hasClass(uncheckedClass)).toBe( + // true + // ); + // }); it('shows as partially selected when at least one but not all tags selected', () => { const wrapper = setup.mount( @@ -396,20 +398,6 @@ describe('NodeList', () => { const elementTypes = Object.keys(sidebarElementTypes); expect(nodeList.length).toBe(tags.length + elementTypes.length); }); - it('renders the correct number of modular pipelines and nodes in the tree sidepanel', () => { - const wrapper = setup.mount( - - - - ); - const nodeList = wrapper.find('.node-list-row__text--tree'); - const modularPipelinesTree = getModularPipelinesTree( - mockState.spaceflights - ); - expect(nodeList.length).toBe( - modularPipelinesTree['__root__'].children.length - ); - }); it('renders elements panel, filter panel inside a SplitPanel with a handle', () => { const wrapper = setup.mount( @@ -435,26 +423,6 @@ describe('NodeList', () => { }); }); - describe('node list element item', () => { - const wrapper = setup.mount( - - - - ); - // this needs to be the 3rd element as the first 2 elements are modular pipelines rows which does not apply the '--active' class - const nodeRow = () => wrapper.find('.node-list-tree-item-row').at(3); - - it('handles mouseenter events', () => { - nodeRow().simulate('mouseenter'); - expect(nodeRow().hasClass('.row--active')).toBe(true); - }); - - it('handles mouseleave events', () => { - nodeRow().simulate('mouseleave'); - expect(nodeRow().hasClass('.row--active')).toBe(false); - }); - }); - describe('node list element item checkbox', () => { const wrapper = setup.mount( diff --git a/src/components/node-list/styles/node-list.scss b/src/components/node-list/styles/node-list.scss index e63f9146a5..d0fab9aa17 100644 --- a/src/components/node-list/styles/node-list.scss +++ b/src/components/node-list/styles/node-list.scss @@ -81,12 +81,38 @@ } } +// Root class for overwriting styles of the pipeline tree item .pipeline-treeItem__root--overwrite { + position: relative; + .Mui-selected { - background-color: transparent !important; + background-color: transparent !important; // Override default background color } + // // Style for tree item content .MuiTreeItem-content { padding: 0; } + + // When hovering over the tree item content, which represents the modular pipeline node + .MuiTreeItem-content:hover { + background-color: var(--color-nodelist-row-active); + + // Change the color of the sibling .MuiTreeItem-group, which contains all nodes belonging to that modular pipeline + ~ .MuiTreeItem-group { + background-color: var(--color-nodelist-row-active); + position: relative; + + // To create the background shadow on the left + &::after { + content: ''; + position: absolute; + left: -40px; + top: 0; + height: 100%; // Match the height of the parent + width: 50px; + background-color: var(--color-nodelist-row-active); + } + } + } } From 864b6cd5440fa2b6e1f1239dea601fd0260bed35 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 23 Oct 2024 15:36:10 +0100 Subject: [PATCH 28/46] update classnames in cypress test Signed-off-by: Huong Nguyen --- cypress/tests/ui/flowchart/flowchart.cy.js | 2 +- cypress/tests/ui/flowchart/menu.cy.js | 26 +++++++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cypress/tests/ui/flowchart/flowchart.cy.js b/cypress/tests/ui/flowchart/flowchart.cy.js index a2eee6fbde..f3f241dcb3 100644 --- a/cypress/tests/ui/flowchart/flowchart.cy.js +++ b/cypress/tests/ui/flowchart/flowchart.cy.js @@ -70,7 +70,7 @@ describe('Flowchart DAG', () => { const nodeToToggleText = 'Parameters'; // Alias - cy.get(`.pipeline-nodelist__row__checkbox[name=${nodeToToggleText}]`).as( + cy.get(`.toggle-control__checkbox[name=${nodeToToggleText}]`).as( 'nodeToToggle' ); diff --git a/cypress/tests/ui/flowchart/menu.cy.js b/cypress/tests/ui/flowchart/menu.cy.js index cba8f32528..ead7607037 100644 --- a/cypress/tests/ui/flowchart/menu.cy.js +++ b/cypress/tests/ui/flowchart/menu.cy.js @@ -41,7 +41,7 @@ describe('Flowchart Menu', () => { }); // Pipeline Label in the Menu - cy.get('.node-list-row__label') + cy.get('.row-text__label') .first() .invoke('text') .should((pipelineLabel) => { @@ -57,7 +57,7 @@ describe('Flowchart Menu', () => { cy.get('.search-input__field').type(searchInput, { force: true }); // Pipeline Label in the Menu - cy.get('.node-list-row__label') + cy.get('.row-text__label') .first() .invoke('text') .should((pipelineLabel) => { @@ -72,7 +72,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `.MuiTreeItem-label > .node-list-row > [data-test=nodelist-data-${nodeToClickText}]` + `.MuiTreeItem-label > .node-list-tree-item-row > [data-test=nodelist-data-${nodeToClickText}]` ) .should('exist') .as('nodeToClick'); @@ -91,7 +91,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `.MuiTreeItem-label > .node-list-row > [data-test=nodelist-data-${nodeToHighlightText}]` + `.MuiTreeItem-label > .node-list-tree-item-row > [data-test=nodelist-data-${nodeToHighlightText}]` ) .should('exist') .as('nodeToHighlight'); @@ -108,7 +108,7 @@ describe('Flowchart Menu', () => { const nodeToToggleText = 'Companies'; // Alias - cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`, { + cy.get(`.toggle-control__checkbox[name=${nodeToToggleText}]`, { timeout: 5000, }).as('nodeToToggle'); @@ -121,7 +121,7 @@ describe('Flowchart Menu', () => { // Assert after action cy.__checkForText__( - `[data-test=nodelist-data-${nodeToToggleText}] > .node-list-row__label--faded`, + `[data-test=nodelist-data-${nodeToToggleText}] > .row-text__label--faded`, nodeToToggleText ); cy.get('.pipeline-node__text').should('not.contain', nodeToToggleText); @@ -137,7 +137,7 @@ describe('Flowchart Menu', () => { // Action cy.get( - `[for=${nodeToFocusText}-focus] > .node-list-row__icon` + `[for=${nodeToFocusText}-focus] > row__icon` ).click(); // Assert after action @@ -161,14 +161,14 @@ describe('Flowchart Menu', () => { const visibleRowLabel = 'Companies'; // Alias - cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`).as( + cy.get(`.toggle-control__checkbox[name=${nodeToToggleText}]`).as( 'nodeToToggle' ); // Assert before action cy.get('@nodeToToggle').should('be.checked'); cy.get( - `[data-test=nodelist-data-${visibleRowLabel}] > .node-list-row__label` + `[data-test=nodelist-data-${visibleRowLabel}] > .row-text__label` ) .should('not.have.class', 'pipeline-nodelist__row__label--faded') .should('not.have.class', 'pipeline-nodelist__row__label--disabled'); @@ -178,7 +178,7 @@ describe('Flowchart Menu', () => { // Assert after action cy.get( - `[data-test=nodelist-data-${visibleRowLabel}] > .node-list-row__label` + `[data-test=nodelist-data-${visibleRowLabel}] > .row-text__label` ) .should('have.class', 'pipeline-nodelist__row__label--faded') .should('have.class', 'pipeline-nodelist__row__label--disabled'); @@ -188,7 +188,7 @@ describe('Flowchart Menu', () => { const nodeToToggleText = 'Parameters'; // Alias - cy.get(`.node-list-row__checkbox[name=${nodeToToggleText}]`).as( + cy.get(`.toggle-control__checkbox[name=${nodeToToggleText}]`).as( 'nodeToToggle' ); @@ -207,7 +207,7 @@ describe('Flowchart Menu', () => { cy.visit(`/?tags=${visibleRowLabel}`); // Alias - cy.get(`.node-list-row__checkbox[name=${visibleRowLabel}]`).as( + cy.get(`.toggle-control__checkbox[name=${visibleRowLabel}]`).as( 'nodeToToggle' ); @@ -220,7 +220,7 @@ describe('Flowchart Menu', () => { cy.visit('/?types=datasets'); // Alias - cy.get(`.node-list-row__checkbox[name=${visibleRowLabel}]`).as( + cy.get(`.toggle-control__checkbox[name=${visibleRowLabel}]`).as( 'nodeToToggle' ); From 96b9199757e0dbd4b85428683837781ca29b1234 Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 23 Oct 2024 20:05:57 +0100 Subject: [PATCH 29/46] Styling for hovering and focus mode Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.scss | 6 +-- .../node-list/components/row/row.js | 3 -- .../node-list/components/row/row.scss | 45 +++-------------- .../node-list/node-list-tree-item.js | 5 +- .../node-list/styles/node-list.scss | 50 ++++++++++++++----- src/components/ui/row-text/row-text.js | 7 +-- src/components/ui/row-text/row-text.scss | 5 ++ tools/test-lib/react-app/app.test.js | 4 +- 8 files changed, 62 insertions(+), 63 deletions(-) diff --git a/src/components/filter-row/filter-row.scss b/src/components/filter-row/filter-row.scss index c66e44aa06..e5cae1a6f4 100644 --- a/src/components/filter-row/filter-row.scss +++ b/src/components/filter-row/filter-row.scss @@ -6,12 +6,12 @@ } .filter-row { - position: relative; - display: flex; align-items: center; - height: 32px; // Fixed row height required for lazy list, apply any changes to filter-row.js. background-color: initial; cursor: default; + display: flex; + height: 32px; + position: relative; &--kind-filter { padding: 0 variables.$row-offset-right 0 variables.$row-offset-left; diff --git a/src/components/node-list/components/row/row.js b/src/components/node-list/components/row/row.js index 99086ac2a2..d609eb01c5 100755 --- a/src/components/node-list/components/row/row.js +++ b/src/components/node-list/components/row/row.js @@ -1,5 +1,4 @@ import React from 'react'; -import { connect } from 'react-redux'; import { uniqueId } from 'lodash'; import classnames from 'classnames'; import NodeIcon from '../../../icons/node-icon'; @@ -8,7 +7,6 @@ import InvisibleIcon from '../../../icons/invisible'; import FocusModeIcon from '../../../icons/focus-mode'; import { ToggleControl } from '../../../ui/toggle-control/toggle-control'; import { RowText } from '../../../ui/row-text/row-text'; -import { getNodeActive } from '../../../../selectors/nodes'; import './row.scss'; @@ -50,7 +48,6 @@ const Row = ({ className={classnames('row kedro', `row--kind-${kind}`, parentClassName, { 'row--active': active, 'row--selected': selected || (!isSlicingPipelineApplied && highlight), - 'row--disabled': disabled, 'row--overwrite': !(active || selected), })} title={name} diff --git a/src/components/node-list/components/row/row.scss b/src/components/node-list/components/row/row.scss index 2e688c9156..2cb974f4c2 100644 --- a/src/components/node-list/components/row/row.scss +++ b/src/components/node-list/components/row/row.scss @@ -6,37 +6,21 @@ } .row { - position: relative; - display: flex; align-items: center; - height: 32px; // Fixed row height required for lazy list, apply any changes to row.js. - transform: translate(0, 0); background-color: initial; cursor: default; + display: flex; + height: 32px; + position: relative; + transform: translate(0, 0); - &--overwrite { - .Mui-selected & { - .kui-theme--dark & { - background-color: var.$slate-200; - } - - .kui-theme--light & { - background-color: var.$white-0; - } - } - } - - &--selected, - &--visible#{&}--selected { + &--selected { // Additional selector required to increase specificity to override previous rule background-color: var(--color-nodelist-row-selected); border-right: 1px solid var.$blue-300; } - &--disabled { - pointer-events: none; - } - + // to ensure the background of the row covers the full width on hover &::before { position: absolute; top: 0; @@ -57,12 +41,6 @@ opacity: 1; } -.row--overwrite::before { - .Mui-selected & { - opacity: 1; - } -} - .row__icon { display: block; flex-shrink: 0; @@ -70,10 +48,6 @@ height: variables.$row-icon-size; fill: var(--color-text); - &.row__toggle-icon--focus-checked { - fill: var.$blue-300; - } - &--disabled > * { opacity: 0.1; } @@ -101,10 +75,3 @@ } } } - -.pipeline-nodelist__elements-panel .MuiTreeItem-label { - // Handle MuiTreeItem icon offset for correct width - $icon-offset: 15px + 4px; - - width: calc(100% - #{$icon-offset}); -} diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index c3e8337b5e..f1b12ee309 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -1,4 +1,5 @@ import React from 'react'; +import classnames from 'classnames'; import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; import ChevronRightIcon from '@mui/icons-material/ChevronRight'; import { TreeItem } from '@mui/x-tree-view'; @@ -18,7 +19,9 @@ const NodeListTreeItem = ({ isSlicingPipelineApplied, }) => ( { */ const testFirstNodeNameMatch = (container, key) => { const firstNodeName = container - .querySelector('.node-list-row') - .querySelector('.node-list-row__label') + .querySelector('.node-list-tree-item-row') + .querySelector('.row-text__label') .textContent.trim(); const modularPipelinesTree = dataSources[key]().modular_pipelines; From ab405779ad858d158778a270caa8e77efc1a739c Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 23 Oct 2024 20:19:56 +0100 Subject: [PATCH 30/46] fixing small styling Signed-off-by: Huong Nguyen --- .../node-list/components/row/row.scss | 4 ++ .../node-list/styles/node-list.scss | 38 +++++-------------- .../ui/toggle-control/toggle-control.js | 2 + .../ui/toggle-control/toggle-control.scss | 8 ++-- 4 files changed, 19 insertions(+), 33 deletions(-) diff --git a/src/components/node-list/components/row/row.scss b/src/components/node-list/components/row/row.scss index 2cb974f4c2..b4cb229e8d 100644 --- a/src/components/node-list/components/row/row.scss +++ b/src/components/node-list/components/row/row.scss @@ -48,6 +48,10 @@ height: variables.$row-icon-size; fill: var(--color-text); + &.toggle-control--icon--focus-checked { + fill: var.$blue-300; + } + &--disabled > * { opacity: 0.1; } diff --git a/src/components/node-list/styles/node-list.scss b/src/components/node-list/styles/node-list.scss index 268a5e9214..0e9d981619 100644 --- a/src/components/node-list/styles/node-list.scss +++ b/src/components/node-list/styles/node-list.scss @@ -5,19 +5,6 @@ @use './section'; @use './variables'; -// Mixin for the ::after pseudo-element -@mixin after-shadow($left, $width, $color) { - &::after { - content: ''; - position: absolute; - left: $left; - top: 0; - height: 100%; // Match the height of the parent - width: $width; - background-color: $color; - } -} - .kui-theme--light { --color-nodelist-row-active: #{variables.$row-active-light}; --color-nodelist-row-selected: #{variables.$row-selected-light}; @@ -116,26 +103,19 @@ position: relative; // Apply the after-shadow mixin to ensure the background covers the full width on hover - @include after-shadow(-40px, 50px, var(--color-nodelist-row-active)); + &::after { + content: ''; + position: absolute; + left: $left; + top: 0; + height: 100%; // Match the height of the parent + width: $width; + background-color: $color; + } } } } -// Focused state for the pipeline tree item -.pipeline-treeItem__root--overwrite--focused { - .MuiTreeItem-content, - .MuiTreeItem-group { - background-color: var( - --color-nodelist-row-active - ); // Change background color - - position: relative; // Ensure .MuiTreeItem-group is positioned relative - - // Apply the after-shadow mixin to ensure the background covers the full width on hover - @include after-shadow(-40px, 50px, var(--color-nodelist-row-active)); - } -} - .pipeline-nodelist__elements-panel .MuiTreeItem-label { // Handle MuiTreeItem icon offset for correct width $icon-offset: 15px + 4px; diff --git a/src/components/ui/toggle-control/toggle-control.js b/src/components/ui/toggle-control/toggle-control.js index 33dfdb4122..5361a3584f 100755 --- a/src/components/ui/toggle-control/toggle-control.js +++ b/src/components/ui/toggle-control/toggle-control.js @@ -8,6 +8,7 @@ export const ToggleControl = ({ className, focusChecked, IconComponent, + dsiabled, id, isChecked, kind, @@ -28,6 +29,7 @@ export const ToggleControl = ({ 'toggle-control--icon--checked': isChecked, 'toggle-control--icon--unchecked': !isChecked, 'toggle-control--icon--focus-checked': focusChecked, + 'toggle-control--icon--disabled': dsiabled, } ); diff --git a/src/components/ui/toggle-control/toggle-control.scss b/src/components/ui/toggle-control/toggle-control.scss index c3ea538e8b..1c31777702 100644 --- a/src/components/ui/toggle-control/toggle-control.scss +++ b/src/components/ui/toggle-control/toggle-control.scss @@ -12,10 +12,6 @@ &--kind-element:nth-of-type(2) { margin: 0 8px 0 -8px; } - - &--disabled { - display: none; - } } @include mixins.transparentColour( @@ -38,6 +34,10 @@ padding: variables.$toggle-icon-padding; border-radius: 50%; + &--disabled { + display: none; + } + .toggle-control__checkbox:focus + & { outline: none; From 608986708cb76a182ab87b574eeea19f0a03ceeb Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Wed, 23 Oct 2024 21:20:20 +0100 Subject: [PATCH 31/46] fix the disable styling for row Signed-off-by: Huong Nguyen --- src/components/node-list/components/row/row.test.js | 5 ----- src/components/node-list/node-list-tree-item.js | 1 + src/components/node-list/styles/node-list.scss | 11 ++++++++--- src/components/ui/row-text/row-text.js | 2 ++ src/components/ui/row-text/row-text.scss | 2 +- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/node-list/components/row/row.test.js b/src/components/node-list/components/row/row.test.js index f26723f006..42294ab8dd 100644 --- a/src/components/node-list/components/row/row.test.js +++ b/src/components/node-list/components/row/row.test.js @@ -55,11 +55,6 @@ describe('Row Component', () => { expect(wrapper.find('.row').hasClass('row--selected')).toBe(true); }); - it('applies the row--disabled class when disabled is true', () => { - const wrapper = setup.mount(); - expect(wrapper.find('.row').hasClass('row--disabled')).toBe(true); - }); - it('applies the overwrite class if not selected or active', () => { const activeNodeWrapper = setup.mount( diff --git a/src/components/node-list/node-list-tree-item.js b/src/components/node-list/node-list-tree-item.js index f1b12ee309..2e217a1b2a 100644 --- a/src/components/node-list/node-list-tree-item.js +++ b/src/components/node-list/node-list-tree-item.js @@ -21,6 +21,7 @@ const NodeListTreeItem = ({ Date: Thu, 24 Oct 2024 09:54:04 +0100 Subject: [PATCH 32/46] fix the disable styling on focus mode Signed-off-by: Huong Nguyen --- src/components/node-list/components/row/row.scss | 4 ---- src/components/ui/toggle-control/toggle-control.js | 4 ++-- src/components/ui/toggle-control/toggle-control.scss | 10 +++++++--- 3 files changed, 9 insertions(+), 9 deletions(-) mode change 100644 => 100755 src/components/node-list/components/row/row.scss diff --git a/src/components/node-list/components/row/row.scss b/src/components/node-list/components/row/row.scss old mode 100644 new mode 100755 index b4cb229e8d..2cb974f4c2 --- a/src/components/node-list/components/row/row.scss +++ b/src/components/node-list/components/row/row.scss @@ -48,10 +48,6 @@ height: variables.$row-icon-size; fill: var(--color-text); - &.toggle-control--icon--focus-checked { - fill: var.$blue-300; - } - &--disabled > * { opacity: 0.1; } diff --git a/src/components/ui/toggle-control/toggle-control.js b/src/components/ui/toggle-control/toggle-control.js index 5361a3584f..968b717daf 100755 --- a/src/components/ui/toggle-control/toggle-control.js +++ b/src/components/ui/toggle-control/toggle-control.js @@ -8,7 +8,7 @@ export const ToggleControl = ({ className, focusChecked, IconComponent, - dsiabled, + disabled, id, isChecked, kind, @@ -29,7 +29,7 @@ export const ToggleControl = ({ 'toggle-control--icon--checked': isChecked, 'toggle-control--icon--unchecked': !isChecked, 'toggle-control--icon--focus-checked': focusChecked, - 'toggle-control--icon--disabled': dsiabled, + 'toggle-control--icon--disabled': disabled, } ); diff --git a/src/components/ui/toggle-control/toggle-control.scss b/src/components/ui/toggle-control/toggle-control.scss index 1c31777702..181770d3ad 100644 --- a/src/components/ui/toggle-control/toggle-control.scss +++ b/src/components/ui/toggle-control/toggle-control.scss @@ -29,13 +29,13 @@ } .toggle-control--icon { - width: variables.$toggle-icon-size; - height: variables.$toggle-icon-size; + width: variables.$toggle-icon-size !important; + height: variables.$toggle-icon-size !important; padding: variables.$toggle-icon-padding; border-radius: 50%; &--disabled { - display: none; + display: none !important; } .toggle-control__checkbox:focus + & { @@ -45,6 +45,10 @@ box-shadow: 0 0 0 3px colors.$blue-300 inset; } } + + &.toggle-control--icon--focus-checked { + fill: colors.$blue-300; + } } // There are two kinds of toggle icon, with different styling: From 29ded2009cd4627f1596011a3a2b0ac6f062b40d Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 24 Oct 2024 10:44:35 +0100 Subject: [PATCH 33/46] remove one of the old test Signed-off-by: Huong Nguyen --- src/components/node-list/node-list.test.js | 25 ---------------------- 1 file changed, 25 deletions(-) diff --git a/src/components/node-list/node-list.test.js b/src/components/node-list/node-list.test.js index d6c3b39318..2118bd9f46 100644 --- a/src/components/node-list/node-list.test.js +++ b/src/components/node-list/node-list.test.js @@ -261,31 +261,6 @@ describe('NodeList', () => { const partialIcon = (wrapper) => tagItem(wrapper).find(IndicatorPartialIcon); - it('selecting tags enables only elements with given tags and modular pipelines', () => { - //Parameters are enabled here to override the default behavior - const wrapper = setup.mount( - - - , - { - beforeLayoutActions: [() => toggleTypeDisabled('parameters', false)], - } - ); - - changeRows(wrapper, ['Preprocessing'], true); - expect(elementsEnabled(wrapper)).toEqual([ - ['data_processing', true], - ['data_science', true], - ]); - - changeRows(wrapper, ['Preprocessing', 'Features'], true); - expect(elementsEnabled(wrapper)).toEqual([ - ['data_processing', true], - ['data_science', true], - ['model_input_table', true], - ]); - }); - it('selecting a tag sorts elements by modular pipelines first then by task, data and parameter nodes ', () => { //Parameters are enabled here to override the default behavior const wrapper = setup.mount( From c53944279cf4b2e17731373690bc899576d52cfe Mon Sep 17 00:00:00 2001 From: Huong Nguyen Date: Thu, 24 Oct 2024 11:14:41 +0100 Subject: [PATCH 34/46] update name for icons for FilterRow Signed-off-by: Huong Nguyen --- src/components/filter-row/filter-row.js | 10 +++++----- src/components/node-list/node-list-group.js | 4 ++-- src/components/node-list/node-list-row-list.js | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/filter-row/filter-row.js b/src/components/filter-row/filter-row.js index a08191eabc..0da21d4274 100755 --- a/src/components/filter-row/filter-row.js +++ b/src/components/filter-row/filter-row.js @@ -1,7 +1,7 @@ import React from 'react'; import classnames from 'classnames'; -import VisibleIcon from '../icons/visible'; -import InvisibleIcon from '../icons/invisible'; +import IndicatorIcon from '../icons/indicator'; +import OffIndicatorIcon from '../icons/indicator-off'; import { ToggleControl } from '../ui/toggle-control/toggle-control'; import { RowText } from '../ui/row-text/row-text'; @@ -14,7 +14,7 @@ export const FilterRow = ({ count, dataTest, id, - invisibleIcon = InvisibleIcon, + offIndicatorIcon = OffIndicatorIcon, kind, label, name, @@ -22,9 +22,9 @@ export const FilterRow = ({ onClick, parentClassName, visible, - visibleIcon = VisibleIcon, + indicatorIcon = IndicatorIcon, }) => { - const Icon = checked ? visibleIcon : invisibleIcon; + const Icon = checked ? indicatorIcon : offIndicatorIcon; return (