From ea33f6273280c9a0544b11a862a673119c0fe4c6 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Wed, 7 Oct 2020 14:17:55 +0100 Subject: [PATCH 01/16] Revert "Gatekeep machines and units panels off production" This reverts commit 06a63e9b142932b35f202e4d03e3f4b4dfd29c40. --- src/pages/Models/Details/ModelDetails.js | 22 ++++++-------------- src/pages/Models/Details/_model-details.scss | 4 ++-- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/pages/Models/Details/ModelDetails.js b/src/pages/Models/Details/ModelDetails.js index 912d895d0..6af55be4e 100644 --- a/src/pages/Models/Details/ModelDetails.js +++ b/src/pages/Models/Details/ModelDetails.js @@ -220,12 +220,10 @@ const ModelDetails = () => { const unitTableRows = useMemo(() => { const handleUnitsRowClick = (e) => { - if (process.env.NODE_ENV !== "production") { - setQuery({ - panel: "units", - entity: e.currentTarget.dataset.unit, - }); - } + setQuery({ + panel: "units", + entity: e.currentTarget.dataset.unit, + }); }; return generateUnitRows( modelStatusData, @@ -237,12 +235,7 @@ const ModelDetails = () => { const machinesTableRows = useMemo(() => { const handleMachineRowClick = (e) => { - if (process.env.NODE_ENV !== "production") { - setQuery({ - panel: "machines", - entity: e.currentTarget.dataset.machine, - }); - } + setQuery({ panel: "machines", entity: e.currentTarget.dataset.machine }); }; return generateMachineRows( modelStatusData, @@ -286,10 +279,7 @@ const ModelDetails = () => {
-
+
{renderCounts(activeView, modelStatusData)} diff --git a/src/pages/Models/Details/_model-details.scss b/src/pages/Models/Details/_model-details.scss index 6d6fcf007..1edb11f03 100644 --- a/src/pages/Models/Details/_model-details.scss +++ b/src/pages/Models/Details/_model-details.scss @@ -190,8 +190,8 @@ } .model-details__apps, - [data-enable-panels="true"] .model-details__machines, - [data-enable-panels="true"] .model-details__units { + .model-details__machines, + .model-details__units { tbody tr:hover { background-color: #e7f9ff; cursor: pointer; From aa288d40b850c2b3baf51cd6ed0abd91703a2f40 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Wed, 7 Oct 2020 15:06:24 +0100 Subject: [PATCH 02/16] Fix typo --- src/components/panels/UnitsPanel/UnitsPanel.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/panels/UnitsPanel/UnitsPanel.js b/src/components/panels/UnitsPanel/UnitsPanel.js index d1fa9cdd2..5d1a895f2 100644 --- a/src/components/panels/UnitsPanel/UnitsPanel.js +++ b/src/components/panels/UnitsPanel/UnitsPanel.js @@ -64,7 +64,7 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { ); }, [app, unit, unitId]); - const machinePanelHeader = useMemo( + const unitsPanelHeader = useMemo( () => generateUnitsPanelHeader(modelStatusData?.applications[unitId]), [modelStatusData, unitId, generateUnitsPanelHeader] ); @@ -80,7 +80,7 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { className="units-panel" > <> - {machinePanelHeader} + {unitsPanelHeader}
Date: Thu, 8 Oct 2020 09:45:02 +0100 Subject: [PATCH 03/16] Add machines table to units panel --- .../panels/UnitsPanel/UnitsPanel.js | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/components/panels/UnitsPanel/UnitsPanel.js b/src/components/panels/UnitsPanel/UnitsPanel.js index 5d1a895f2..304010b3a 100644 --- a/src/components/panels/UnitsPanel/UnitsPanel.js +++ b/src/components/panels/UnitsPanel/UnitsPanel.js @@ -1,12 +1,14 @@ import React, { useMemo, useCallback } from "react"; import SlidePanel from "components/SlidePanel/SlidePanel"; import MainTable from "@canonical/react-components/dist/components/MainTable"; +import cloneDeep from "clone-deep"; import useModelStatus from "hooks/useModelStatus"; import { machineTableHeaders, applicationTableHeaders, + generateMachineRows, } from "pages/Models/Details/generators"; import { generateStatusElement, extractRevisionNumber } from "app/utils"; @@ -19,6 +21,21 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { const unit = modelStatusData?.applications[appName]?.units[unitId]; const app = modelStatusData?.applications[appName]; + const filteredModelStatusDataByMachine = useCallback( + (unit) => { + const filteredModelStatusData = cloneDeep(modelStatusData); + if (unit?.machine) { + Object.keys(filteredModelStatusData.machines).forEach((machineId) => { + if (machineId !== unit.machine) { + delete filteredModelStatusData.machines[machineId]; + } + }); + } + return filteredModelStatusData; + }, + [modelStatusData] + ); + // Generate panel header for given entity const generateUnitsPanelHeader = useCallback(() => { return ( @@ -69,6 +86,12 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { [modelStatusData, unitId, generateUnitsPanelHeader] ); + // Generate machines table + const machineRows = useMemo( + () => generateMachineRows(filteredModelStatusDataByMachine(unit)), + [filteredModelStatusDataByMachine, unit] + ); + // Check for loading status const isLoading = !modelStatusData?.machines; @@ -84,7 +107,7 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) {
Date: Thu, 8 Oct 2020 10:39:28 +0100 Subject: [PATCH 04/16] Add applications table to units panel --- .../panels/UnitsPanel/UnitsPanel.js | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/components/panels/UnitsPanel/UnitsPanel.js b/src/components/panels/UnitsPanel/UnitsPanel.js index 304010b3a..de9b792f6 100644 --- a/src/components/panels/UnitsPanel/UnitsPanel.js +++ b/src/components/panels/UnitsPanel/UnitsPanel.js @@ -9,6 +9,7 @@ import { machineTableHeaders, applicationTableHeaders, generateMachineRows, + generateApplicationRows, } from "pages/Models/Details/generators"; import { generateStatusElement, extractRevisionNumber } from "app/utils"; @@ -36,6 +37,22 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { [modelStatusData] ); + const filteredModelStatusDataByApp = useCallback( + (appName) => { + const filteredModelStatusData = cloneDeep(modelStatusData); + filteredModelStatusData && + Object.keys(filteredModelStatusData.applications).forEach( + (application) => { + if (application !== appName) { + delete filteredModelStatusData.applications[application]; + } + } + ); + return filteredModelStatusData; + }, + [modelStatusData] + ); + // Generate panel header for given entity const generateUnitsPanelHeader = useCallback(() => { return ( @@ -86,12 +103,19 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { [modelStatusData, unitId, generateUnitsPanelHeader] ); - // Generate machines table + // Generate machines table content const machineRows = useMemo( - () => generateMachineRows(filteredModelStatusDataByMachine(unit)), + () => + generateMachineRows(filteredModelStatusDataByMachine(unit, "machines")), [filteredModelStatusDataByMachine, unit] ); + // Generate apps table content + const applicationRows = useMemo( + () => generateApplicationRows(filteredModelStatusDataByApp(appName)), + [filteredModelStatusDataByApp, appName] + ); + // Check for loading status const isLoading = !modelStatusData?.machines; @@ -114,7 +138,7 @@ export default function UnitsPanel({ isActive, onClose, entity: unitId }) { /> Date: Thu, 8 Oct 2020 11:16:48 +0100 Subject: [PATCH 05/16] Add applications tables to the machines panel --- .../panels/MachinesPanel/MachinesPanel.js | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index 79ed17fff..1fb13c576 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -1,12 +1,14 @@ import React, { useMemo, useCallback } from "react"; import SlidePanel from "components/SlidePanel/SlidePanel"; import MainTable from "@canonical/react-components/dist/components/MainTable"; +import cloneDeep from "clone-deep"; import useModelStatus from "hooks/useModelStatus"; import { unitTableHeaders, applicationTableHeaders, + generateApplicationRows, } from "pages/Models/Details/generators"; import { generateStatusElement } from "app/utils"; @@ -86,6 +88,32 @@ export default function MachinesPanel({ [modelStatusData, machineId, generateMachinesPanelHeader] ); + const filteredModelStatusDataByApp = useCallback( + (machineId) => { + const filteredModelStatusData = cloneDeep(modelStatusData); + filteredModelStatusData && + Object.keys(filteredModelStatusData.applications).forEach( + (application) => { + const units = + filteredModelStatusData.applications[application].units; + Object.values(units).forEach((unit) => { + if (unit.machine !== machineId) { + delete filteredModelStatusData.applications[application]; + } + }); + } + ); + return filteredModelStatusData; + }, + [modelStatusData] + ); + + // Generate apps table content + const applicationRows = useMemo( + () => generateApplicationRows(filteredModelStatusDataByApp(machineId)), + [filteredModelStatusDataByApp, machineId] + ); + // Check for loading status const isLoading = !modelStatusData?.machines; @@ -108,7 +136,7 @@ export default function MachinesPanel({ /> Date: Fri, 9 Oct 2020 13:54:23 +0100 Subject: [PATCH 06/16] Active status icons should be green --- src/scss/custom/_status_icons.scss | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/scss/custom/_status_icons.scss b/src/scss/custom/_status_icons.scss index cff73a385..a914ef699 100644 --- a/src/scss/custom/_status_icons.scss +++ b/src/scss/custom/_status_icons.scss @@ -32,13 +32,18 @@ } &.is-running, - &.is-started, - &.is-active { + &.is-started { &::before { color: $color-mid-light; } } + &.is-active { + &::before { + color: $color-positive; + } + } + &.is-unknown { &::before { border: 1px solid $color-mid-light; From 6d0cd6d6b64b345c57ec23f223c2a0b0a59aa1e4 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Fri, 9 Oct 2020 14:55:22 +0100 Subject: [PATCH 07/16] Add a loading spinner to model details --- src/pages/Models/Details/ModelDetails.js | 1 + src/pages/Models/Details/_model-details.scss | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/pages/Models/Details/ModelDetails.js b/src/pages/Models/Details/ModelDetails.js index 6af55be4e..f5d60f245 100644 --- a/src/pages/Models/Details/ModelDetails.js +++ b/src/pages/Models/Details/ModelDetails.js @@ -1,5 +1,6 @@ import React, { useEffect, useMemo } from "react"; import MainTable from "@canonical/react-components/dist/components/MainTable"; +import Spinner from "@canonical/react-components/dist/components/Spinner"; import { useDispatch, useSelector } from "react-redux"; import { useParams } from "react-router-dom"; import { useQueryParams, StringParam, withDefault } from "use-query-params"; diff --git a/src/pages/Models/Details/_model-details.scss b/src/pages/Models/Details/_model-details.scss index 1edb11f03..855c61d89 100644 --- a/src/pages/Models/Details/_model-details.scss +++ b/src/pages/Models/Details/_model-details.scss @@ -9,6 +9,14 @@ padding-bottom: 3rem; padding-top: 1rem; + &__loading { + align-items: center; + display: flex; + justify-items: center; + min-height: calc(100vh - 48px); + width: 100%; + } + @media (min-width: $breakpoint-medium) { gap: 1rem; grid-template-columns: 230px 1fr; From bbe8b1eb30979a25b267d9eed9076d203fb0c0cc Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Mon, 12 Oct 2020 14:28:09 +0100 Subject: [PATCH 08/16] Refactor conditional rendering --- src/app/selectors.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/selectors.js b/src/app/selectors.js index 70e329e0b..fd648d294 100644 --- a/src/app/selectors.js +++ b/src/app/selectors.js @@ -18,7 +18,7 @@ import { @returns {Object|Null} The list of model data or null if none found. */ export const getModelData = (state) => { - if (state.juju && state.juju.modelData) { + if (state?.juju?.modelData) { return state.juju.modelData; } return null; @@ -107,7 +107,7 @@ const getFilteredModelData = (filters) => */ const getUserCredentials = (state) => { let storedMacaroons = null; - if (state.root && state.root.bakery) { + if (state?.root?.bakery) { storedMacaroons = state.root.bakery.storage._store; } return storedMacaroons; From 46e524714234b0d7521eeff96458df9a56eec771 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Mon, 12 Oct 2020 16:48:42 +0100 Subject: [PATCH 09/16] Enable user to navigate across panels --- src/components/panels/AppsPanel/AppsPanel.js | 10 +- .../panels/MachinesPanel/MachinesPanel.js | 38 +++- .../panels/UnitsPanel/UnitsPanel.js | 19 +- src/components/panels/_panels.scss | 4 - src/pages/Models/Details/ModelDetails.js | 196 ++++++++++-------- src/pages/Models/Details/_model-details.scss | 30 ++- src/pages/Models/Details/generators.js | 9 +- 7 files changed, 179 insertions(+), 127 deletions(-) diff --git a/src/components/panels/AppsPanel/AppsPanel.js b/src/components/panels/AppsPanel/AppsPanel.js index 45576f1a4..1d0a47308 100644 --- a/src/components/panels/AppsPanel/AppsPanel.js +++ b/src/components/panels/AppsPanel/AppsPanel.js @@ -24,7 +24,7 @@ import { import "./_apps-panel.scss"; -export default function AppsPanel({ isActive, onClose, entity }) { +export default function AppsPanel({ isActive, onClose, entity, panelRowClick }) { // Get model status info const modelStatusData = useModelStatus(); @@ -105,13 +105,13 @@ export default function AppsPanel({ isActive, onClose, entity }) { ); const machinesPanelRows = useMemo( - () => generateMachineRows(filteredModelStatusData), - [filteredModelStatusData] + () => generateMachineRows(filteredModelStatusData, panelRowClick), + [filteredModelStatusData, panelRowClick] ); const unitPanelRows = useMemo( - () => generateUnitRows(filteredModelStatusData, baseAppURL), - [baseAppURL, filteredModelStatusData] + () => generateUnitRows(filteredModelStatusData, panelRowClick), + [panelRowClick, filteredModelStatusData] ); const relationPanelRows = useMemo( diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index 1fb13c576..f539e74d3 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -7,6 +7,7 @@ import useModelStatus from "hooks/useModelStatus"; import { unitTableHeaders, + generateUnitRows, applicationTableHeaders, generateApplicationRows, } from "pages/Models/Details/generators"; @@ -19,6 +20,7 @@ export default function MachinesPanel({ isActive, onClose, entity: machineId, + panelRowClick }) { const modelStatusData = useModelStatus(); const machine = modelStatusData?.machines[machineId]; @@ -108,10 +110,38 @@ export default function MachinesPanel({ [modelStatusData] ); + const filteredModelStatusDataByUnit = useCallback( + (machineId) => { + const filteredModelStatusData = cloneDeep(modelStatusData); + filteredModelStatusData && + Object.keys(filteredModelStatusData.applications).forEach( + (application) => { + const units = + filteredModelStatusData.applications[application].units; + for (let [key, unit] of Object.entries(units)) { + if (unit.machine !== machineId) { + delete filteredModelStatusData.applications[application].units[ + key + ]; + } + } + } + ); + return filteredModelStatusData; + }, + [modelStatusData] + ); + // Generate apps table content const applicationRows = useMemo( - () => generateApplicationRows(filteredModelStatusDataByApp(machineId)), - [filteredModelStatusDataByApp, machineId] + () => generateApplicationRows(filteredModelStatusDataByApp(machineId), panelRowClick), + [filteredModelStatusDataByApp, machineId, panelRowClick] + ); + + // Generate units table content + const unitRows = useMemo( + () => generateUnitRows(filteredModelStatusDataByUnit(machineId), panelRowClick), + [filteredModelStatusDataByUnit, machineId, panelRowClick] ); // Check for loading status @@ -129,14 +159,14 @@ export default function MachinesPanel({
- generateMachineRows(filteredModelStatusDataByMachine(unit, "machines")), - [filteredModelStatusDataByMachine, unit] + generateMachineRows(filteredModelStatusDataByMachine(unit, "machines"), panelRowClick), + [filteredModelStatusDataByMachine, panelRowClick, unit] ); // Generate apps table content const applicationRows = useMemo( - () => generateApplicationRows(filteredModelStatusDataByApp(appName)), - [filteredModelStatusDataByApp, appName] + () => + generateApplicationRows( + filteredModelStatusDataByApp(appName), + panelRowClick + ), + [filteredModelStatusDataByApp, panelRowClick, appName] ); // Check for loading status diff --git a/src/components/panels/_panels.scss b/src/components/panels/_panels.scss index d79016951..7e40316eb 100644 --- a/src/components/panels/_panels.scss +++ b/src/components/panels/_panels.scss @@ -54,8 +54,4 @@ grid-template-columns: repeat(2, 1fr); } } - - &__table tr { - pointer-events: none; - } } diff --git a/src/pages/Models/Details/ModelDetails.js b/src/pages/Models/Details/ModelDetails.js index f5d60f245..7a0b44fe5 100644 --- a/src/pages/Models/Details/ModelDetails.js +++ b/src/pages/Models/Details/ModelDetails.js @@ -262,6 +262,10 @@ const ModelDetails = () => { const { panel: activePanel, entity, activeView } = query; const closePanelConfig = { panel: undefined, entity: undefined }; + const panelRowClick = (e, entityName, entityPanel) => { + return setQuery({ panel: entityPanel, entity: entityName }); + }; + return (
@@ -270,107 +274,121 @@ const ModelDetails = () => { {modelStatusData ? modelStatusData.model.name : "..."}
- + {modelStatusData && ( + + )}
-
-
- -
- {renderCounts(activeView, modelStatusData)} - {shouldShow("apps", activeView) && ( - - )} - {shouldShow("units", activeView) && ( - - )} - {shouldShow("machines", activeView) && ( - - )} - {shouldShow("relations", activeView) && ( - <> - {shouldShow("relations-title", activeView) && ( -
Relations ({relationTableRows.length})
+ {!modelStatusData ? ( +
+ +
+ ) : ( +
+
+ +
+ {renderCounts(activeView, modelStatusData)} + {shouldShow("apps", activeView) && + applicationTableRows.length > 0 && ( + )} + {shouldShow("units", activeView) && unitTableRows.length > 0 && ( - {shouldShow("relations-title", activeView) && ( -
- Cross-model relations ( - {consumedTableRows.length + offersTableRows.length}) -
- )} - {consumedTableRows.length ? ( - - ) : null} - {offersTableRows.length ? ( + )} + {shouldShow("machines", activeView) && + machinesTableRows.length > 0 && ( - ) : null} - - )} + )} + {shouldShow("relations", activeView) && + relationTableRows.length > 0 && ( + <> + {shouldShow("relations-title", activeView) && ( +
Relations ({relationTableRows.length})
+ )} + + {shouldShow("relations-title", activeView) && ( +
+ Cross-model relations ( + {consumedTableRows.length + offersTableRows.length}) +
+ )} + {consumedTableRows.length ? ( + + ) : null} + {offersTableRows.length ? ( + + ) : null} + + )} +
+ setQuery(closePanelConfig)} + panelRowClick={panelRowClick} + /> + setQuery(closePanelConfig)} + panelRowClick={panelRowClick} + /> + setQuery(closePanelConfig)} + panelRowClick={panelRowClick} + />
- setQuery(closePanelConfig)} - /> - setQuery(closePanelConfig)} - /> - setQuery(closePanelConfig)} - /> -
+ )} {generateTerminalComponent(modelUUID, controllerWSHost)} ); diff --git a/src/pages/Models/Details/_model-details.scss b/src/pages/Models/Details/_model-details.scss index 855c61d89..a84615521 100644 --- a/src/pages/Models/Details/_model-details.scss +++ b/src/pages/Models/Details/_model-details.scss @@ -57,23 +57,21 @@ } } -@mixin model-details-main { - .model-details__main { - .subordinate-row { - border-top: none !important; - } +@mixin model-details-subordinates { + .subordinate-row { + border-top: none !important; + } - .subordinate { - margin-right: 0.5rem; - padding-left: 1.5rem; - position: relative; + .subordinate { + margin-right: 0.5rem; + padding-left: 1.5rem; + position: relative; - &::before { - content: url("../../../static/images/unit-tree.svg"); - left: 0.75rem; - position: absolute; - top: -0.25rem; - } + &::before { + content: url("../../../static/images/unit-tree.svg"); + left: 0.75rem; + position: absolute; + top: -0.25rem; } } } @@ -222,6 +220,6 @@ @include model-details-layout; @include model-details-header; @include model-details-title; -@include model-details-main; +@include model-details-subordinates; @include model-details-tables; @include model-details-entity-icons; diff --git a/src/pages/Models/Details/generators.js b/src/pages/Models/Details/generators.js index a421fc9d9..1712cf11d 100644 --- a/src/pages/Models/Details/generators.js +++ b/src/pages/Models/Details/generators.js @@ -183,7 +183,7 @@ export function generateApplicationRows( os: "Ubuntu", notes: "-", }, - onClick: (e) => onRowClick(e, app), + onClick: (e) => onRowClick(e, key, "apps"), "data-app": key, className: selectedEntity === key ? "is-selected" : "", }; @@ -250,7 +250,7 @@ export function generateUnitRows( port, message, }, - onClick: (e) => onRowClick(e, unitId), + onClick: (e) => onRowClick(e, unitId, "units"), "data-unit": unitId, className: selectedEntity === unitId ? "is-selected" : "", }); @@ -267,7 +267,8 @@ export function generateUnitRows( subordinate.charm, key, true, - baseAppURL + baseAppURL, + true // disable link ), className: "u-truncate", }, @@ -366,7 +367,7 @@ export function generateMachineRows( instanceId: machine.instanceId, message: machine?.agentStatus?.info, }, - onClick: (e) => onRowClick(e, machineId), + onClick: (e) => onRowClick(e, machineId, "machines"), "data-machine": machineId, className: selectedEntity === machineId ? "is-selected" : "", }; From 32fdb8395560a40650b3280c08cda5b390f42936 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Mon, 12 Oct 2020 17:22:40 +0100 Subject: [PATCH 10/16] Fix up Prettier --- src/components/panels/AppsPanel/AppsPanel.js | 7 ++++++- src/components/panels/MachinesPanel/MachinesPanel.js | 11 ++++++++--- src/components/panels/UnitsPanel/UnitsPanel.js | 5 ++++- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/components/panels/AppsPanel/AppsPanel.js b/src/components/panels/AppsPanel/AppsPanel.js index 1d0a47308..d687d219c 100644 --- a/src/components/panels/AppsPanel/AppsPanel.js +++ b/src/components/panels/AppsPanel/AppsPanel.js @@ -24,7 +24,12 @@ import { import "./_apps-panel.scss"; -export default function AppsPanel({ isActive, onClose, entity, panelRowClick }) { +export default function AppsPanel({ + isActive, + onClose, + entity, + panelRowClick, +}) { // Get model status info const modelStatusData = useModelStatus(); diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index f539e74d3..d4cd578f5 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -20,7 +20,7 @@ export default function MachinesPanel({ isActive, onClose, entity: machineId, - panelRowClick + panelRowClick, }) { const modelStatusData = useModelStatus(); const machine = modelStatusData?.machines[machineId]; @@ -134,13 +134,18 @@ export default function MachinesPanel({ // Generate apps table content const applicationRows = useMemo( - () => generateApplicationRows(filteredModelStatusDataByApp(machineId), panelRowClick), + () => + generateApplicationRows( + filteredModelStatusDataByApp(machineId), + panelRowClick + ), [filteredModelStatusDataByApp, machineId, panelRowClick] ); // Generate units table content const unitRows = useMemo( - () => generateUnitRows(filteredModelStatusDataByUnit(machineId), panelRowClick), + () => + generateUnitRows(filteredModelStatusDataByUnit(machineId), panelRowClick), [filteredModelStatusDataByUnit, machineId, panelRowClick] ); diff --git a/src/components/panels/UnitsPanel/UnitsPanel.js b/src/components/panels/UnitsPanel/UnitsPanel.js index 1e9176d0a..08c0195bc 100644 --- a/src/components/panels/UnitsPanel/UnitsPanel.js +++ b/src/components/panels/UnitsPanel/UnitsPanel.js @@ -111,7 +111,10 @@ export default function UnitsPanel({ // Generate machines table content const machineRows = useMemo( () => - generateMachineRows(filteredModelStatusDataByMachine(unit, "machines"), panelRowClick), + generateMachineRows( + filteredModelStatusDataByMachine(unit, "machines"), + panelRowClick + ), [filteredModelStatusDataByMachine, panelRowClick, unit] ); From 18475131391e506e4ac814e2f0e3e84367317015 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Mon, 12 Oct 2020 21:50:00 +0100 Subject: [PATCH 11/16] Fix tests --- src/pages/Models/Details/ModelDetails.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pages/Models/Details/ModelDetails.test.js b/src/pages/Models/Details/ModelDetails.test.js index 5158155b0..a75a9e629 100644 --- a/src/pages/Models/Details/ModelDetails.test.js +++ b/src/pages/Models/Details/ModelDetails.test.js @@ -31,7 +31,7 @@ describe("ModelDetail Container", () => { ); expect(wrapper.find("Topology").length).toBe(1); - expect(wrapper.find(".model-details__main table").length).toBe(4); + expect(wrapper.find(".model-details__main table").length).toBe(2); }); it("renders the details pane for models shared-with-me", () => { @@ -216,7 +216,9 @@ describe("ModelDetail Container", () => { expect( wrapper.find(".slide-panel.machines-panel").prop("aria-hidden") ).toBe(true); - const machineRow = wrapper.find(`tr[data-machine="${testMachine}"]`); + const machineRow = wrapper.find( + `.model-details__main tr[data-machine="${testMachine}"]` + ); machineRow.simulate("click"); expect( wrapper.find(".slide-panel.machines-panel").prop("aria-hidden") From 2635a0ea8fbdfac4816e8e216341d53ea04ca36e Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Mon, 12 Oct 2020 22:02:25 +0100 Subject: [PATCH 12/16] Refactor and cleanup --- src/pages/Models/Details/ModelDetails.js | 41 ++++++++---------------- src/pages/Models/Details/generators.js | 6 ++-- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/src/pages/Models/Details/ModelDetails.js b/src/pages/Models/Details/ModelDetails.js index 7a0b44fe5..358d60375 100644 --- a/src/pages/Models/Details/ModelDetails.js +++ b/src/pages/Models/Details/ModelDetails.js @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo } from "react"; +import React, { useEffect, useMemo, useCallback } from "react"; import MainTable from "@canonical/react-components/dist/components/MainTable"; import Spinner from "@canonical/react-components/dist/components/Spinner"; import { useDispatch, useSelector } from "react-redux"; @@ -192,6 +192,13 @@ const ModelDetails = () => { setQuery({ activeView: view }); }; + const panelRowClick = useCallback( + (entityName, entityPanel) => { + return setQuery({ panel: entityPanel, entity: entityName }); + }, + [setQuery] + ); + useEffect(() => { dispatch(collapsibleSidebar(true)); return () => { @@ -208,42 +215,26 @@ const ModelDetails = () => { }, [dispatch, modelUUID, modelStatusData]); const applicationTableRows = useMemo(() => { - const handleAppRowClick = (e) => { - setQuery({ panel: "apps", entity: e.currentTarget.dataset.app }); - }; return generateApplicationRows( modelStatusData, - handleAppRowClick, + panelRowClick, baseAppURL, query?.entity ); - }, [baseAppURL, modelStatusData, setQuery, query]); + }, [baseAppURL, modelStatusData, query, panelRowClick]); const unitTableRows = useMemo(() => { - const handleUnitsRowClick = (e) => { - setQuery({ - panel: "units", - entity: e.currentTarget.dataset.unit, - }); - }; return generateUnitRows( modelStatusData, - handleUnitsRowClick, + panelRowClick, baseAppURL, query?.entity ); - }, [baseAppURL, modelStatusData, query, setQuery]); + }, [baseAppURL, modelStatusData, query, panelRowClick]); const machinesTableRows = useMemo(() => { - const handleMachineRowClick = (e) => { - setQuery({ panel: "machines", entity: e.currentTarget.dataset.machine }); - }; - return generateMachineRows( - modelStatusData, - handleMachineRowClick, - query?.entity - ); - }, [modelStatusData, setQuery, query]); + return generateMachineRows(modelStatusData, panelRowClick, query?.entity); + }, [modelStatusData, panelRowClick, query]); const relationTableRows = useMemo( () => generateRelationRows(modelStatusData, baseAppURL), @@ -262,10 +253,6 @@ const ModelDetails = () => { const { panel: activePanel, entity, activeView } = query; const closePanelConfig = { panel: undefined, entity: undefined }; - const panelRowClick = (e, entityName, entityPanel) => { - return setQuery({ panel: entityPanel, entity: entityName }); - }; - return (
diff --git a/src/pages/Models/Details/generators.js b/src/pages/Models/Details/generators.js index 1712cf11d..e46518222 100644 --- a/src/pages/Models/Details/generators.js +++ b/src/pages/Models/Details/generators.js @@ -183,7 +183,7 @@ export function generateApplicationRows( os: "Ubuntu", notes: "-", }, - onClick: (e) => onRowClick(e, key, "apps"), + onClick: () => onRowClick(key, "apps"), "data-app": key, className: selectedEntity === key ? "is-selected" : "", }; @@ -250,7 +250,7 @@ export function generateUnitRows( port, message, }, - onClick: (e) => onRowClick(e, unitId, "units"), + onClick: () => onRowClick(unitId, "units"), "data-unit": unitId, className: selectedEntity === unitId ? "is-selected" : "", }); @@ -367,7 +367,7 @@ export function generateMachineRows( instanceId: machine.instanceId, message: machine?.agentStatus?.info, }, - onClick: (e) => onRowClick(e, machineId, "machines"), + onClick: () => onRowClick(machineId, "machines"), "data-machine": machineId, className: selectedEntity === machineId ? "is-selected" : "", }; From 58a48f22a0d0827a8e2bfd28c1114c8eee2d1eff Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Wed, 14 Oct 2020 11:07:22 +0100 Subject: [PATCH 13/16] Delete apps without units from machines panel --- .../panels/MachinesPanel/MachinesPanel.js | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index d4cd578f5..3c08f9a45 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -97,12 +97,20 @@ export default function MachinesPanel({ Object.keys(filteredModelStatusData.applications).forEach( (application) => { const units = - filteredModelStatusData.applications[application].units; - Object.values(units).forEach((unit) => { - if (unit.machine !== machineId) { - delete filteredModelStatusData.applications[application]; - } - }); + filteredModelStatusData.applications[application]?.units; + + if (Object.entries(units).length) { + Object.values(units).forEach((unit) => { + if ( + unit.machine !== machineId || + !Object.entries(units).length + ) { + delete filteredModelStatusData.applications[application]; + } + }); + } else { + delete filteredModelStatusData.applications[application]; + } } ); return filteredModelStatusData; From 205206f22ffa8ce11dfdc4594e3f28114be77772 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Wed, 14 Oct 2020 11:22:58 +0100 Subject: [PATCH 14/16] Switch deps in useMemo to match function signature --- src/components/panels/AppsPanel/AppsPanel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/panels/AppsPanel/AppsPanel.js b/src/components/panels/AppsPanel/AppsPanel.js index d687d219c..61b22b85f 100644 --- a/src/components/panels/AppsPanel/AppsPanel.js +++ b/src/components/panels/AppsPanel/AppsPanel.js @@ -116,7 +116,7 @@ export default function AppsPanel({ const unitPanelRows = useMemo( () => generateUnitRows(filteredModelStatusData, panelRowClick), - [panelRowClick, filteredModelStatusData] + [filteredModelStatusData, panelRowClick] ); const relationPanelRows = useMemo( From bcd8a391251dbb8c4956c6c0c612a67ac52fe35f Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Fri, 16 Oct 2020 09:44:48 +0100 Subject: [PATCH 15/16] Add comment to explain deleting applications without units --- src/components/panels/MachinesPanel/MachinesPanel.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index 3c08f9a45..7b66f981b 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -102,7 +102,9 @@ export default function MachinesPanel({ if (Object.entries(units).length) { Object.values(units).forEach((unit) => { if ( + // Delete any app without a unit matching this machineId... unit.machine !== machineId || + // ...delete any app without units at all !Object.entries(units).length ) { delete filteredModelStatusData.applications[application]; From 6d65f9eb1b7d37b55dbe95f1b84042d7b37c4e91 Mon Sep 17 00:00:00 2001 From: Barry McGee Date: Fri, 16 Oct 2020 10:20:51 +0100 Subject: [PATCH 16/16] Wrap all model detail panels in one slide panel --- src/components/panels/AppsPanel/AppsPanel.js | 70 +++++++------------ .../panels/MachinesPanel/MachinesPanel.js | 58 ++++++--------- .../panels/MachinesPanel/_machines-panel.scss | 2 +- .../panels/UnitsPanel/UnitsPanel.js | 49 +++++-------- src/pages/Models/Details/ModelDetails.js | 34 ++++----- src/pages/Models/Details/ModelDetails.test.js | 24 ++----- 6 files changed, 91 insertions(+), 146 deletions(-) diff --git a/src/components/panels/AppsPanel/AppsPanel.js b/src/components/panels/AppsPanel/AppsPanel.js index 61b22b85f..55eb40cbc 100644 --- a/src/components/panels/AppsPanel/AppsPanel.js +++ b/src/components/panels/AppsPanel/AppsPanel.js @@ -1,7 +1,6 @@ import React, { useMemo } from "react"; import { useSelector } from "react-redux"; import { getConfig } from "app/selectors"; -import SlidePanel from "components/SlidePanel/SlidePanel"; import MainTable from "@canonical/react-components/dist/components/MainTable"; import useModelStatus from "hooks/useModelStatus"; @@ -24,12 +23,7 @@ import { import "./_apps-panel.scss"; -export default function AppsPanel({ - isActive, - onClose, - entity, - panelRowClick, -}) { +export default function AppsPanel({ entity, panelRowClick }) { // Get model status info const modelStatusData = useModelStatus(); @@ -124,42 +118,32 @@ export default function AppsPanel({ [filteredModelStatusData, baseAppURL] ); - // Check for loading status - const isLoading = !filteredModelStatusData?.applications?.[entity]; - return ( - - <> - {appPanelHeader} -
- - - -
- - + <> + {appPanelHeader} +
+ + + +
+ ); } diff --git a/src/components/panels/MachinesPanel/MachinesPanel.js b/src/components/panels/MachinesPanel/MachinesPanel.js index 7b66f981b..bce046ba8 100644 --- a/src/components/panels/MachinesPanel/MachinesPanel.js +++ b/src/components/panels/MachinesPanel/MachinesPanel.js @@ -1,5 +1,4 @@ import React, { useMemo, useCallback } from "react"; -import SlidePanel from "components/SlidePanel/SlidePanel"; import MainTable from "@canonical/react-components/dist/components/MainTable"; import cloneDeep from "clone-deep"; @@ -16,12 +15,7 @@ import { generateStatusElement } from "app/utils"; import "./_machines-panel.scss"; -export default function MachinesPanel({ - isActive, - onClose, - entity: machineId, - panelRowClick, -}) { +export default function MachinesPanel({ entity: machineId, panelRowClick }) { const modelStatusData = useModelStatus(); const machine = modelStatusData?.machines[machineId]; @@ -43,7 +37,7 @@ export default function MachinesPanel({ {machine && (
-
+
Machine '{machineId}' - {machine?.series} @@ -159,35 +153,25 @@ export default function MachinesPanel({ [filteredModelStatusDataByUnit, machineId, panelRowClick] ); - // Check for loading status - const isLoading = !modelStatusData?.machines; - return ( - - <> - {machinePanelHeader} -
- - -
- - + <> + {machinePanelHeader} +
+ + +
+ ); } diff --git a/src/components/panels/MachinesPanel/_machines-panel.scss b/src/components/panels/MachinesPanel/_machines-panel.scss index b4962c8e8..7e1550cae 100644 --- a/src/components/panels/MachinesPanel/_machines-panel.scss +++ b/src/components/panels/MachinesPanel/_machines-panel.scss @@ -1,6 +1,6 @@ @import "../panels"; -.machine-panel { +.machines-panel { &__id { padding-left: 2rem; text-transform: capitalize; diff --git a/src/components/panels/UnitsPanel/UnitsPanel.js b/src/components/panels/UnitsPanel/UnitsPanel.js index 08c0195bc..87950658a 100644 --- a/src/components/panels/UnitsPanel/UnitsPanel.js +++ b/src/components/panels/UnitsPanel/UnitsPanel.js @@ -1,5 +1,4 @@ import React, { useMemo, useCallback } from "react"; -import SlidePanel from "components/SlidePanel/SlidePanel"; import MainTable from "@canonical/react-components/dist/components/MainTable"; import cloneDeep from "clone-deep"; @@ -128,35 +127,25 @@ export default function UnitsPanel({ [filteredModelStatusDataByApp, panelRowClick, appName] ); - // Check for loading status - const isLoading = !modelStatusData?.machines; - return ( - - <> - {unitsPanelHeader} -
- - -
- - + <> + {unitsPanelHeader} +
+ + +
+ ); } diff --git a/src/pages/Models/Details/ModelDetails.js b/src/pages/Models/Details/ModelDetails.js index 358d60375..62826305c 100644 --- a/src/pages/Models/Details/ModelDetails.js +++ b/src/pages/Models/Details/ModelDetails.js @@ -11,6 +11,7 @@ import InfoPanel from "components/InfoPanel/InfoPanel"; import Layout from "components/Layout/Layout"; import Header from "components/Header/Header"; import Terminal from "components/Terminal/Terminal"; +import SlidePanel from "components/SlidePanel/SlidePanel"; import AppsPanel from "components/panels/AppsPanel/AppsPanel"; import MachinesPanel from "components/panels/MachinesPanel/MachinesPanel"; @@ -356,24 +357,23 @@ const ModelDetails = () => { )}
- setQuery(closePanelConfig)} - panelRowClick={panelRowClick} - /> - setQuery(closePanelConfig)} - panelRowClick={panelRowClick} - /> - setQuery(closePanelConfig)} - panelRowClick={panelRowClick} - /> + isLoading={!entity} + className={`${activePanel}-panel`} + > + {activePanel === "apps" && ( + + )} + {activePanel === "machines" && ( + + )} + {activePanel === "units" && ( + + )} +
)} {generateTerminalComponent(modelUUID, controllerWSHost)} diff --git a/src/pages/Models/Details/ModelDetails.test.js b/src/pages/Models/Details/ModelDetails.test.js index a75a9e629..1af3e9663 100644 --- a/src/pages/Models/Details/ModelDetails.test.js +++ b/src/pages/Models/Details/ModelDetails.test.js @@ -186,14 +186,10 @@ describe("ModelDetail Container", () => { ); - expect(wrapper.find(".slide-panel.apps-panel").prop("aria-hidden")).toBe( - true - ); + expect(wrapper.find(".slide-panel.apps-panel").length).toBe(0); const applicationRow = wrapper.find(`tr[data-app="${testApp}"]`); applicationRow.simulate("click"); - expect(wrapper.find(".slide-panel.apps-panel").prop("aria-hidden")).toBe( - false - ); + expect(wrapper.find(".slide-panel.apps-panel").length).toBe(1); expect( wrapper.find(".slide-panel.apps-panel .panel-header .entity-name").text() ).toBe("kibana"); @@ -213,16 +209,12 @@ describe("ModelDetail Container", () => { ); - expect( - wrapper.find(".slide-panel.machines-panel").prop("aria-hidden") - ).toBe(true); + expect(wrapper.find(".slide-panel.machines-panel").length).toBe(0); const machineRow = wrapper.find( `.model-details__main tr[data-machine="${testMachine}"]` ); machineRow.simulate("click"); - expect( - wrapper.find(".slide-panel.machines-panel").prop("aria-hidden") - ).toBe(false); + expect(wrapper.find(".slide-panel.machines-panel").length).toBe(1); expect( wrapper .find(".slide-panel.machines-panel .panel-header .entity-name") @@ -244,14 +236,10 @@ describe("ModelDetail Container", () => { ); - expect(wrapper.find(".slide-panel.units-panel").prop("aria-hidden")).toBe( - true - ); + expect(wrapper.find(".slide-panel.units-panel").length).toBe(0); const unitRow = wrapper.find(`tr[data-unit="${testUnit}"]`); unitRow.simulate("click"); - expect(wrapper.find(".slide-panel.units-panel").prop("aria-hidden")).toBe( - false - ); + expect(wrapper.find(".slide-panel.units-panel").length).toBe(1); expect( wrapper.find(".slide-panel.units-panel .panel-header .entity-name").text() ).toBe("kibana/0");