Merge remote-tracking branch 'origin/master' into gui-v2-qrcode

.github/workflows/ci.yml

@@ -5,13 +5,13 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
-      - name: Setup Node.js 18.x
+      - uses: actions/checkout@v4
+      - name: Setup Node.js 20.x
         uses: actions/setup-node@master
         with:
-          node-version: 18.x
+          node-version: 20.x
       - name: Node modules cache
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ~/.npm
           key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
@@ -30,30 +30,30 @@ jobs:
           --name venus-docker victronenergy/venus-docker:latest
           /root/run_with_simulation.sh z
       - name: Run unit and E2E tests
-        run: npm run test:ci || exit 0
+        run: npm run test:ci
       - name: Generate E2E Report
         run: npm run generate-e2e-report
         if: always()
       - name: Upload E2E Results
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: e2e-tests-results
           path: cypress/results/results.json
       - name: Upload E2E Report
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: e2e-tests-report
           path: cypress/report
       - name: Upload Cypress Screenshots
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: cypress-screenshots
           path: cypress/screenshots
       - name: Upload Cypress Videos
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v4
         if: always()
         with:
           name: cypress-videos
@@ -91,4 +91,4 @@ jobs:
           upload_url: ${{ steps.create_release.outputs.upload_url }}
           asset_path: ./venus-html5-app.tar.gz
           asset_name: venus-html5-app.tar.gz
-          asset_content_type: application/zip
+          asset_content_type: application/zip

.gitignore

@@ -26,6 +26,7 @@ yarn-debug.log*
 yarn-error.log*
 
 cypress/results
+cypress/screenshots
 cypress/videos
 cypress/report
 

README.md

@@ -71,11 +71,13 @@ And then open the app in the browser at `http://localhost:8000`.
 
 This will start the webpack dev server, which will recompile the app on code changes and hot reload the UI.
 
-You can change the `host`, `port`, and `path` (defaults to `websocket-mqtt`) query parameters to connect to a different Venus websocket MQTT host.
+Note that the app will attempt to by default connect to MQTT broker served via the same port as the app and path `websocket-mqtt`, and that will eventually fail.
+
+You will need to change the `host`, `port`, and `path` (defaults to `websocket-mqtt`) query parameters to connect to a different Venus websocket MQTT host.
 
 To connect to a Venus device with `VENUS_DEVICE_IP` running firmware >= 3.50 use the following URL:
 
-`http://localhost:8000?host=<VENUS_DEVICE_IP>`
+`http://localhost:8000?host=<VENUS_DEVICE_IP>&port=80`
 
 To connect to Venus device with `VENUS_DEVICE_IP` running firmware < 3.50, or to a `venus-docker` simulation, use the following URL:
 
@@ -181,11 +183,11 @@ Most components have Enzyme unit tests. Run all of these tests with `npm run tes
 Cypress is used to run integration tests on the compiled ui to make sure it opens and operated correctly in different
 display sizes. To run cypress you need to run the live server and an instance of venus docker in the Venus GX demo mode (z):
 
-(in html5 app repo): `npm run dev`
+(in html5 app repo): `npm run start`
 
 (in venus docker repo): `./run.sh -s z`
 
-Then you can run the cypress ui with `npm run cypress`.
+Then you can run the cypress UI interactively with `npm run cy:open`.
 
 To run the ui tests in CI-style use `npm run test:e2e`
 

cypress.config.ts

@@ -15,7 +15,7 @@ export default defineConfig({
     json: true,
   },
   e2e: {
-    baseUrl: "http://localhost:8000/",
+    baseUrl: "http://localhost:8000/?host=localhost&port=9001&path=%02%03",
     specPattern: "cypress/e2e/**/*.{js,jsx,ts,tsx}",
   },
 })

package.json

@@ -78,7 +78,7 @@
     "sass": "^1.49.8",
     "sass-loader": "^10.0.5",
     "semver": "^7.5.2",
-    "serve": "^14.1.2",
+    "serve": "^14.2.3",
     "stream-browserify": "^3.0.0",
     "terser-webpack-plugin": "^4.2.3",
     "ts-pnp": "^1.2.0",
@@ -146,13 +146,13 @@
     "start": "node scripts/start.js",
     "build": "node scripts/build.js",
     "test": "node scripts/test.js --config ./jest.config.js",
-    "serve": "./node_modules/.bin/serve -C -s -l 8000 dist",
+    "serve": "./node_modules/.bin/serve -C -l 8000 ./",
     "test:dev": "npm test -- --watchAll=false && npm run cy:run",
     "test:unit": "npm test",
     "test:e2e": "npm run cy:run",
     "test:ci": "npm test -- --watchAll=false && npm run cy:run",
     "generate-e2e-report": "npm run copy-e2e-assets && npm run merge-e2e-results && npm run create-e2e-html-report",
-    "copy-e2e-assets": "rm -rf ./cypress/report && mkdir ./cypress/report && cp -r ./cypress/screenshots ./cypress/videos ./cypress/report/",
+    "copy-e2e-assets": "rm -rf ./cypress/report && mkdir -p ./cypress/screenshots && mkdir -p ./cypress/videos && mkdir ./cypress/report && cp -r ./cypress/screenshots ./cypress/videos ./cypress/report/",
     "merge-e2e-results": "./node_modules/.bin/mochawesome-merge ./cypress/results/*.json -o ./cypress/results/results.json",
     "create-e2e-html-report": "./node_modules/.bin/marge ./cypress/results/results.json -f index.html -o ./cypress/report --cdn",
     "cy:version": "./node_modules/.bin/cypress version",

serve.json

@@ -0,0 +1,8 @@
+{
+  "public": "./dist",
+  "rewrites": [
+    { "source": "app/:a", "destination": ":a" },
+    { "source": "app/:a/:b", "destination": ":a/:b" },
+    { "source": "app/:a/:b/:c", "destination": ":a/:b/:c" }
+  ]
+}

src/app/Marine2/components/boxes/BatteriesOverview/BatteriesOverview.tsx

@@ -32,7 +32,9 @@ const BatteriesOverview = ({ componentMode = "full", pageSelectorPropsSetter }:
 
   const { temperatureUnitToHumanReadable } = useAppStore()
 
-  useVisibilityNotifier({ widgetName: BOX_TYPES.BATTERIES, visible: !!(batteries && batteries.length) })
+  const hasValidData = !!(batteries && batteries.length)
+
+  useVisibilityNotifier({ widgetName: BOX_TYPES.BATTERIES, isVisible: hasValidData })
 
   const sortedBatteries = sortBatteries(batteries ?? [])
   const overviewBatteries = batteriesForOverview(sortedBatteries)

src/app/Marine2/components/boxes/DevicesOverview/DevicesOverview.tsx

@@ -32,6 +32,11 @@ import { ISize } from "@m2Types/generic/size"
 import { AC_SOURCE, BOX_TYPES } from "../../../utils/constants/generic"
 import { RELAY_FUNCTION } from "../../../utils/constants/devices/generators"
 
+interface Props {
+  componentMode?: ComponentMode
+  pageSelectorPropsSetter?: (arg0: PageSelectorProps) => void
+}
+
 const DevicesOverview = ({ componentMode = "full", pageSelectorPropsSetter }: Props) => {
   const { inverters } = useInverters()
   const { instanceId: vebusInstanceId, vebusInverters } = useVebus()
@@ -51,7 +56,9 @@ const DevicesOverview = ({ componentMode = "full", pageSelectorPropsSetter }: Pr
     componentMode
   )
 
-  useVisibilityNotifier({ widgetName: BOX_TYPES.DEVICES, visible: !!boxes.length })
+  const hasValidData = !!boxes.length
+
+  useVisibilityNotifier({ widgetName: BOX_TYPES.DEVICES, isVisible: hasValidData })
 
   if (!boxes.length) {
     return null
@@ -186,9 +193,4 @@ const getAvailableDeviceBoxes = function (
   return devices
 }
 
-interface Props {
-  componentMode?: ComponentMode
-  pageSelectorPropsSetter?: (arg0: PageSelectorProps) => void
-}
-
 export default observer(DevicesOverview)

src/app/Marine2/components/boxes/EnergyOverview/EnergyOverview.tsx

@@ -27,8 +27,9 @@ const EnergyOverview: FC<Props> = ({ componentMode = "full", pageSelectorPropsSe
   const boxes = useAvailableEnergyBoxes(compactBoxSize, componentMode)
   const activeStyles = applyStyles(compactBoxSize, defaultBoxStyles)
 
-  // TODO: it seems that visibility logic can be improved since the energy component always has an overview box
-  useVisibilityNotifier({ widgetName: BOX_TYPES.ENERGY, visible: boxes.length > 0 })
+  const hasValidData = boxes.length > 0
+
+  useVisibilityNotifier({ widgetName: BOX_TYPES.ENERGY, isVisible: hasValidData })
 
   if (!boxes.length) {
     return null

src/app/Marine2/components/boxes/EnvironmentOverview/EnvironmentOverview.tsx

@@ -72,7 +72,9 @@ const EnvironmentOverview = ({ componentMode = "full", pageSelectorPropsSetter }
     (sensor: any) => sensor.temperature || sensor.humidity || sensor.pressure
   )
 
-  useVisibilityNotifier({ widgetName: BOX_TYPES.ENVIRONMENT, visible: sensorHasData })
+  const hasValidData = sensorHasData
+
+  useVisibilityNotifier({ widgetName: BOX_TYPES.ENVIRONMENT, isVisible: hasValidData })
 
   if (componentMode === "compact") {
     return (

src/app/Marine2/components/boxes/Tanks/Tanks.tsx

@@ -27,7 +27,9 @@ const Tanks = ({ componentMode = "full", className }: Props) => {
 
   const [boxSize, setBoxSize] = useState<ISize>({ width: 0, height: 0 })
 
-  useVisibilityNotifier({ widgetName: BOX_TYPES.TANKS, visible: !!filteredTanks.length })
+  const hasValidData = !!filteredTanks.length
+
+  useVisibilityNotifier({ widgetName: BOX_TYPES.TANKS, isVisible: hasValidData })
 
   const gridRef = useRef<HTMLDivElement>(null)
   const [orientation, setOrientation] = useState<ScreenOrientation>("vertical")

src/app/Marine2/components/views/RootView.tsx

@@ -21,10 +21,12 @@ const RootView = () => {
     const visibleBoxes: JSX.Element[] = []
     const hiddenBoxes: JSX.Element[] = []
     for (const type of Object.values(BOX_TYPES)) {
+      const isVisible = visibleWidgetsStore.visibleElements.has(type)
+
       const elem = getBoxByType(type)
       if (!elem) continue
 
-      if (visibleWidgetsStore.visibleElements.has(type)) {
+      if (isVisible) {
         visibleBoxes.push(elem)
       } else {
         hiddenBoxes.push(elem)
@@ -33,7 +35,7 @@ const RootView = () => {
 
     setBoxes(visibleBoxes)
     setInitialBoxes(hiddenBoxes)
-  }, [visibleWidgetsStore.visibleElements, visibleWidgetsStore.visibleElements.size])
+  }, [visibleWidgetsStore.visibleElements])
 
   const getBoxByType = (type: BOX_TYPES) => {
     switch (type) {

src/app/Marine2/modules/MetricsWidgets/VisibleWidgets.provider.ts

@@ -1,13 +1,12 @@
 import { useEffect } from "react"
-import { notifyParams, useVisibleWidgetsStore } from "./VisibleWidgets.store"
+import { VisibilityParams, useVisibleWidgetsStore } from "./VisibleWidgets.store"
 
-export const useVisibilityNotifier = ({ widgetName, visible }: notifyParams) => {
+export const useVisibilityNotifier = ({ widgetName, isVisible }: VisibilityParams) => {
   const visibleWidgetsStore = useVisibleWidgetsStore()
 
   useEffect(() => {
-    visibleWidgetsStore.notifyVisibility({ widgetName, visible })
+    visibleWidgetsStore.changeVisibility({ widgetName, isVisible })
 
-    return () => visibleWidgetsStore.notifyVisibility({ widgetName, visible: false })
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [visible, widgetName])
+  }, [widgetName, isVisible])
 }

src/app/Marine2/modules/MetricsWidgets/VisibleWidgets.store.ts

@@ -2,33 +2,42 @@ import { makeAutoObservable } from "mobx"
 import { useMemo } from "react"
 import { BOX_TYPES } from "../../utils/constants/generic"
 
-export type notifyParams = {
+export type VisibilityParams = {
   widgetName: BOX_TYPES
-  visible: boolean
+  isVisible: boolean
 }
 
 export class VisibleWidgets {
-  // TODO: add order prop to have a consistent order of boxes
   visibleElements: Set<BOX_TYPES> = new Set()
 
   constructor() {
     makeAutoObservable(this)
   }
 
-  notifyVisibility(element: notifyParams) {
-    if (element.visible) {
+  changeVisibility(element: VisibilityParams) {
+    let isDirty = false
+    if (element.isVisible && !this.visibleElements.has(element.widgetName)) {
       this.visibleElements.add(element.widgetName)
-    } else {
+      isDirty = true
+    } else if (!element.isVisible && this.visibleElements.has(element.widgetName)) {
       this.visibleElements.delete(element.widgetName)
+      isDirty = true
+    }
+
+    // React compares complex types like Set or Array by reference
+    // when computing dependency changes.
+    // Duplicate the Set when its elements change to indicate we should re-render.
+    if (isDirty) {
+      this.visibleElements = new Set(this.visibleElements)
     }
   }
 
   get noVisibleElements() {
-    return !this.visibleElements.size
+    return this.visibleElements.size === 0
   }
 
   clearVisibleElements() {
-    this.visibleElements.clear()
+    this.visibleElements = new Set()
   }
 }