From 92860cd3a9c922abafd3d81ff767cbc2266e993b Mon Sep 17 00:00:00 2001 From: Sajid Alam <90610031+SajidAlamQB@users.noreply.github.com> Date: Mon, 22 Apr 2024 10:48:07 +0100 Subject: [PATCH] Move the `Expand all modular pipelines` toggle from the settings panel (#1858) * Update config.js Signed-off-by: Sajid Alam * Update config.js Signed-off-by: Sajid Alam * remove experiment and expand modular pipeline from settings menu Signed-off-by: Sajid Alam * implement the expandallpipelines Signed-off-by: Sajid Alam * Update flowchart-primary-toolbar.test.js Signed-off-by: Sajid Alam * improve tests Signed-off-by: Sajid Alam * changes based on review Signed-off-by: Sajid Alam * remove query from settings-modal Signed-off-by: Sajid Alam * Update settings-modal.js Signed-off-by: Sajid Alam * design changes make icon always hoverable Signed-off-by: Sajid Alam * update css Signed-off-by: Sajid Alam --------- Signed-off-by: Sajid Alam --- cypress/tests/ui/toolbar/global-toolbar.cy.js | 36 ++---- src/components/app/app.js | 1 + .../run-details-modal/run-details-modal.scss | 12 ++ .../flowchart-primary-toolbar.js | 121 ++++++++++++------ .../flowchart-primary-toolbar.test.js | 23 +++- .../global-toolbar/global-toolbar.scss | 4 + .../global-toolbar/global-toolbar.test.js | 1 + src/components/icons/collapse-pipelines.js | 9 ++ src/components/icons/expand-pipelines.js | 9 ++ .../settings-modal/settings-modal.js | 74 +++++------ src/components/ui/toggle/toggle.scss | 5 +- src/config.js | 6 - src/store/initial-state.js | 1 + src/store/initial-state.test.js | 1 + 14 files changed, 184 insertions(+), 119 deletions(-) create mode 100644 src/components/icons/collapse-pipelines.js create mode 100644 src/components/icons/expand-pipelines.js diff --git a/cypress/tests/ui/toolbar/global-toolbar.cy.js b/cypress/tests/ui/toolbar/global-toolbar.cy.js index 95f07b127d..a0bcc80e91 100644 --- a/cypress/tests/ui/toolbar/global-toolbar.cy.js +++ b/cypress/tests/ui/toolbar/global-toolbar.cy.js @@ -146,38 +146,24 @@ describe('Global Toolbar', () => { }); }); - it('verifies that users can expand all modular pipelines on first load. #TC-7', () => { + it('verifies that users can expand all modular pipelines directly from the toolbar. #TC-7', () => { const modularPipelineChildNodeText = 'Create Derived Features'; - // Alias - cy.get('[data-test="pipeline-toggle-input-expandAllPipelines"]').as( - 'isExpandAllPipelinesCheckBox' - ); + // Alias for better readability + cy.get('[data-test="expand-all-pipelines-toggle"]').as('expandAllPipelinesToggle'); // Assert before action - cy.get('@isExpandAllPipelinesCheckBox').should('not.be.checked'); - cy.get('.pipeline-node__text').should( - 'not.contain', - modularPipelineChildNodeText - ); - cy.get('[role="treeitem"]') - .should('have.attr', 'aria-expanded') - .should('eq', 'false'); + cy.get('@expandAllPipelinesToggle').should('not.be.checked'); + cy.get('.pipeline-node__text').should('not.contain', modularPipelineChildNodeText); + cy.get('[role="treeitem"]').should('have.attr', 'aria-expanded', 'false'); - // Action - cy.get('@isExpandAllPipelinesCheckBox').check({ force: true }); - cy.get('[data-test="Apply changes and close in Settings Modal"]').click({ - force: true, - }); + // Action - toggling the expand all pipelines directly from the toolbar + cy.get('@expandAllPipelinesToggle').click(); // Assert after action - cy.get('[role="treeitem"]', { timeout: 5000 }) - .should('have.attr', 'aria-expanded') - .should('eq', 'true'); - cy.get('.pipeline-node__text').should( - 'contain', - modularPipelineChildNodeText - ); + cy.get('[role="treeitem"]') + .should('have.attr', 'aria-expanded', 'true'); + cy.get('.pipeline-node__text').should('contain', modularPipelineChildNodeText); }); }); }); diff --git a/src/components/app/app.js b/src/components/app/app.js index 6948da6558..ba70cfb8af 100644 --- a/src/components/app/app.js +++ b/src/components/app/app.js @@ -97,6 +97,7 @@ App.propTypes = { labelBtn: PropTypes.bool, layerBtn: PropTypes.bool, exportBtn: PropTypes.bool, + pipelineBtn: PropTypes.bool, sidebar: PropTypes.bool, }), /** diff --git a/src/components/experiment-tracking/run-details-modal/run-details-modal.scss b/src/components/experiment-tracking/run-details-modal/run-details-modal.scss index ed6b438fbc..c43b8b4dae 100644 --- a/src/components/experiment-tracking/run-details-modal/run-details-modal.scss +++ b/src/components/experiment-tracking/run-details-modal/run-details-modal.scss @@ -18,3 +18,15 @@ text-align: right; width: 100%; } + +.version-reminder-and-run-details-button-wrapper { + align-items: baseline; + display: flex; + justify-content: space-between; + width: 100%; + margin-top: 50px; + + .button:first-of-type { + margin-right: 20px; + } +} diff --git a/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.js b/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.js index 2d7932fc57..844fc7ec90 100644 --- a/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.js +++ b/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.js @@ -5,13 +5,18 @@ import { toggleLayers, toggleSidebar, toggleTextLabels, + changeFlag, } from '../../actions'; +import { loadInitialPipelineData } from '../../actions/pipelines'; import IconButton from '../ui/icon-button'; import LabelIcon from '../icons/label'; import ExportIcon from '../icons/export'; import LayersIcon from '../icons/layers'; import PrimaryToolbar from '../primary-toolbar'; import { getVisibleLayerIDs } from '../../selectors/disabled'; +import ExpandPipelinesIcon from '../icons/expand-pipelines'; +import CollapsePipelinesIcon from '../icons/collapse-pipelines'; +import { useGeneratePathname } from '../../utils/hooks/use-generate-pathname'; /** * Main controls for filtering the chart data @@ -28,47 +33,76 @@ export const FlowchartPrimaryToolbar = ({ textLabels, visible, visibleLayers, -}) => ( - <> - - onToggleTextLabels(!textLabels)} - visible={visible.labelBtn} - /> - onToggleLayers(!visibleLayers)} - visible={visible.layerBtn} - /> - onToggleExportModal(true)} - visible={visible.exportBtn} - /> - - -); + expandedPipelines, + onToggleExpandAllPipelines, +}) => { + const { toSetQueryParam } = useGeneratePathname(); + + const handleToggleExpandAllPipelines = () => { + const isExpanded = !expandedPipelines; + onToggleExpandAllPipelines(isExpanded); + toSetQueryParam('expandAllPipelines', isExpanded.toString()); + }; + + return ( + <> + + onToggleTextLabels(!textLabels)} + visible={visible.labelBtn} + /> + onToggleLayers(!visibleLayers)} + visible={visible.layerBtn} + /> + + onToggleExportModal(true)} + visible={visible.exportBtn} + /> + + + ); +}; export const mapStateToProps = (state) => ({ disableLayerBtn: !state.layer.ids.length, @@ -76,6 +110,7 @@ export const mapStateToProps = (state) => ({ textLabels: state.textLabels, visible: state.visible, visibleLayers: Boolean(getVisibleLayerIDs(state).length), + expandedPipelines: state.flags.expandAllPipelines, }); export const mapDispatchToProps = (dispatch) => ({ @@ -91,6 +126,10 @@ export const mapDispatchToProps = (dispatch) => ({ onToggleTextLabels: (value) => { dispatch(toggleTextLabels(Boolean(value))); }, + onToggleExpandAllPipelines: (isExpanded) => { + dispatch(changeFlag('expandAllPipelines', isExpanded)); + dispatch(loadInitialPipelineData()); + }, }); export default connect( diff --git a/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.test.js b/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.test.js index f9c332ca2e..732389f33c 100644 --- a/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.test.js +++ b/src/components/flowchart-primary-toolbar/flowchart-primary-toolbar.test.js @@ -6,10 +6,16 @@ import ConnectedFlowchartPrimaryToolbar, { } from './flowchart-primary-toolbar'; import { mockState, setup } from '../../utils/state.mock'; +jest.mock('../../utils/hooks/use-generate-pathname', () => ({ + useGeneratePathname: () => ({ + toSetQueryParam: jest.fn(), + }), +})); + describe('PrimaryToolbar', () => { it('renders without crashing', () => { const wrapper = setup.mount(); - expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(4); + expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(5); }); it('hides all buttons (except menu button) when visible prop is false for each of them', () => { @@ -17,6 +23,7 @@ describe('PrimaryToolbar', () => { labelBtn: false, layerBtn: false, exportBtn: false, + pipelineBtn: false, }; const wrapper = setup.mount(, { visible, @@ -31,7 +38,7 @@ describe('PrimaryToolbar', () => { const wrapper = setup.mount(, { visible, }); - expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(3); + expect(wrapper.find('.pipeline-icon-toolbar__button').length).toBe(4); }); const functionCalls = [ @@ -39,6 +46,7 @@ describe('PrimaryToolbar', () => { ['.pipeline-menu-button--labels', 'onToggleTextLabels'], ['.pipeline-menu-button--export', 'onToggleExportModal'], ['.pipeline-menu-button--layers', 'onToggleLayers'], + ['.pipeline-menu-button--pipeline', 'onToggleExpandAllPipelines'], ]; test.each(functionCalls)( @@ -70,6 +78,7 @@ describe('PrimaryToolbar', () => { settingsModal: expect.any(Boolean), labelBtn: expect.any(Boolean), layerBtn: expect.any(Boolean), + pipelineBtn: expect.any(Boolean), sidebar: expect.any(Boolean), }), visibleLayers: expect.any(Boolean), @@ -113,5 +122,15 @@ describe('PrimaryToolbar', () => { type: 'TOGGLE_TEXT_LABELS', }); }); + + it('onToggleExpandAllPipelines', () => { + const dispatch = jest.fn(); + mapDispatchToProps(dispatch).onToggleExpandAllPipelines(true); + expect(dispatch.mock.calls[0][0]).toEqual({ + name: 'expandAllPipelines', + type: 'CHANGE_FLAG', + value: true, + }); + }); }); }); diff --git a/src/components/global-toolbar/global-toolbar.scss b/src/components/global-toolbar/global-toolbar.scss index 5a0cb30971..b117e90442 100644 --- a/src/components/global-toolbar/global-toolbar.scss +++ b/src/components/global-toolbar/global-toolbar.scss @@ -46,6 +46,10 @@ } } +.pipeline-menu-button--pipeline svg { + opacity: 0.7; +} + .pipeline-global-routes-toolbar a.active .pipeline-menu-button--link { background-color: var(--color-global-toolbar-active-btn); border-right: 1px solid var(--color-border-line); diff --git a/src/components/global-toolbar/global-toolbar.test.js b/src/components/global-toolbar/global-toolbar.test.js index f591299f08..74e9012844 100644 --- a/src/components/global-toolbar/global-toolbar.test.js +++ b/src/components/global-toolbar/global-toolbar.test.js @@ -56,6 +56,7 @@ describe('GlobalToolbar', () => { miniMapBtn: true, modularPipelineFocusMode: null, metadataModal: false, + pipelineBtn: true, settingsModal: false, shareableUrlModal: false, sidebar: true, diff --git a/src/components/icons/collapse-pipelines.js b/src/components/icons/collapse-pipelines.js new file mode 100644 index 0000000000..e23687c878 --- /dev/null +++ b/src/components/icons/collapse-pipelines.js @@ -0,0 +1,9 @@ +import React from 'react'; + +const CollapsePipelinesIcon = ({ className }) => ( + + + +); + +export default CollapsePipelinesIcon; diff --git a/src/components/icons/expand-pipelines.js b/src/components/icons/expand-pipelines.js new file mode 100644 index 0000000000..d280fa2638 --- /dev/null +++ b/src/components/icons/expand-pipelines.js @@ -0,0 +1,9 @@ +import React from 'react'; + +const ExpandPipelinesIcon = ({ className }) => ( + + + +); + +export default ExpandPipelinesIcon; diff --git a/src/components/settings-modal/settings-modal.js b/src/components/settings-modal/settings-modal.js index c3f081f8e3..a4847d04e7 100644 --- a/src/components/settings-modal/settings-modal.js +++ b/src/components/settings-modal/settings-modal.js @@ -8,14 +8,9 @@ import { } from '../../actions'; import { getFlagsState } from '../../utils/flags'; import SettingsModalRow from './settings-modal-row'; -import { - settings as settingsConfig, - localStorageName, - params, -} from '../../config'; +import { settings as settingsConfig, localStorageName } from '../../config'; import { saveLocalStorage } from '../../store/helpers'; import { localStorageKeyFeatureHintsStep } from '../../components/feature-hints/feature-hints'; -import { useGeneratePathname } from '../../utils/hooks/use-generate-pathname'; import Button from '../ui/button'; import Modal from '../ui/modal'; @@ -47,8 +42,6 @@ const SettingsModal = ({ useState(showFeatureHints); const [toggleFlags, setToggleFlags] = useState(flags); - const { toSetQueryParam } = useGeneratePathname(); - useEffect(() => { setShowFeatureHintsValue(showFeatureHints); }, [showFeatureHints]); @@ -66,9 +59,6 @@ const SettingsModal = ({ const updatedFlags = Object.entries(toggleFlags); updatedFlags.map((each) => { const [name, value] = each; - if (name === params.expandAll) { - toSetQueryParam(params.expandAll, value); - } return onToggleFlag(name, value); }); @@ -95,7 +85,6 @@ const SettingsModal = ({ onToggleIsPrettyName, showSettingsModal, toggleFlags, - toSetQueryParam, ]); const resetStateCloseModal = () => { @@ -115,7 +104,6 @@ const SettingsModal = ({ >
-
General
Name
State
@@ -149,9 +137,6 @@ const SettingsModal = ({ } }} /> -
-
-
Experiments
{flagData.map(({ name, value, description }) => ( ))} +
+
{isRunningLocally() ? ( isOutdated ? (
@@ -190,33 +177,34 @@ const SettingsModal = ({
) ) : null} -
-
- - +
+ + +
diff --git a/src/components/ui/toggle/toggle.scss b/src/components/ui/toggle/toggle.scss index cbb1af584e..c7c900600a 100644 --- a/src/components/ui/toggle/toggle.scss +++ b/src/components/ui/toggle/toggle.scss @@ -8,8 +8,8 @@ } .kui-theme--dark { - --color-toggle-on: #{colors.$blue-300}; - --color-toggle-on-bar: #{colors.$ocean-400}; + --color-toggle-on: #{colors.$blue-0}; + --color-toggle-on-bar: #{colors.$blue-0}; --color-toggle-off: #{colors.$white-0}; --color-toggle-off-bar: #{colors.$black-400}; } @@ -75,6 +75,7 @@ .pipeline-toggle-label--checked::before { background-color: var(--color-toggle-on-bar); + opacity: 0.3; } .pipeline-toggle-label--checked::after { diff --git a/src/config.js b/src/config.js index 0dca77dcd1..9de91e6263 100644 --- a/src/config.js +++ b/src/config.js @@ -63,12 +63,6 @@ export const flags = { default: true, icon: '🐳', }, - expandAllPipelines: { - name: 'Expand all modular pipelines', - description: 'Expand all modular pipelines on first load', - default: false, - icon: '🔛', - }, }; export const settings = { diff --git a/src/store/initial-state.js b/src/store/initial-state.js index 5279bdd49b..8bc03021ea 100644 --- a/src/store/initial-state.js +++ b/src/store/initial-state.js @@ -40,6 +40,7 @@ export const createInitialState = () => ({ miniMap: true, miniMapBtn: true, modularPipelineFocusMode: null, + pipelineBtn: true, settingsModal: false, shareableUrlModal: false, sidebar: window.innerWidth > sidebarWidth.breakpoint, diff --git a/src/store/initial-state.test.js b/src/store/initial-state.test.js index 240bfabd54..56b3458e93 100644 --- a/src/store/initial-state.test.js +++ b/src/store/initial-state.test.js @@ -123,6 +123,7 @@ describe('getInitialState', () => { exportBtn: true, labelBtn: true, layerBtn: true, + pipelineBtn: true, }, }); });