Modernize ofborg-viewer by zebreus · Pull Request #28 · NixOS/ofborg-viewer

zebreus · 2024-02-10T18:37:46Z

This PR modernizes the stack used by ofborg-viewer and automates deployments. It also adds darkmode. You can view a deployed version with this PR at ofborg.wirs.ing (deployed via GH actions to GH pages from my fork).

Changes:

Use flakes with a recent version of nixpkgs
Use nodejs 20 instead of nodejs 6
Use buildNpmPackage instead of yarn2nix
Switch from webpack to vite (because updating from webpack 2 was too much work)
Update eslint & add prettier
Simplify deployment with github actions and github pages
Uses normal css instead of less
Use darkmode if the user prefers-color-scheme: dark

Deployment should be as easy as merging this PR, setting GH pages to deploy from actions, and pointing logs.ofborg.org to GH pages.

Copilot

Pull request overview

This PR modernizes ofborg-viewer’s frontend/tooling and Nix packaging to enable a newer Node/Vite-based build, flake-based Nix workflows, and GitHub Pages deployment, while also migrating styling to CSS with color-scheme support.

Changes:

Replace the legacy webpack/yarn2nix-based toolchain with Vite + npm (Node 20) and add a package-lock.json.
Introduce Nix flakes + buildNpmPackage packaging and add a GitHub Actions workflow to build/deploy to GitHub Pages.
Migrate Less styling to CSS variables and add light/dark theme support via prefers-color-scheme.

Reviewed changes

Copilot reviewed 28 out of 39 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
yarn2nix.json	Removed legacy yarn2nix pin (no longer needed).
webpack/revision.js	Removed webpack-era revision/version helper.
webpack.config.js	Removed webpack build configuration.
vite.config.js	Added Vite config, including build output layout and build-time defines.
src/styles/index.less	Removed Less-based styles.
src/styles/index.css	Added CSS-variable-based styling and color-scheme support.
src/state.js	Reformatted state/history management code.
src/mixins/eventable.js	Reformatted EventTarget-like mixin.
src/listener.js	Migrated STOMP client usage to newer `@stomp/stompjs` API.
src/lib/ready.js	Reformatted DOM-ready helper.
src/lib/html.js	Reformatted HTML parsing helper.
src/lib/bsod.js	Reformatted BSOD rendering helper.
src/index.js	Start app on DOM-ready and keep fetch compatibility shim.
src/index.html	Added Vite-style HTML entrypoint and external CSS link.
src/gui/log.js	Reformatted log UI component.
src/gui/index.js	Reformatted GUI controller and event wiring.
src/config.js	Converted config to named exports (ESM style).
src/backlog.js	Reformatted backlog URL helper.
src/app.js	Adjusted app boot sequencing to align with new entrypoint behavior.
src/app.html	Removed webpack templated HTML entry.
release.nix	Removed legacy release derivation using yarn2nix.
package.json	Updated scripts/deps for Vite/npm + modern eslint/prettier.
package-lock.json	Added npm lockfile for reproducible npm installs.
flake.nix	Added flake entrypoint for Nix builds.
flake.lock	Added pinned nixpkgs lockfile for flakes.
default.nix	Switched to `buildNpmPackage` (Node 20) and produce `dist` output.
bin/yarn2nix-update.sh	Removed yarn2nix update helper script.
bin/webpack-dev-server	Removed webpack dev-server wrapper.
bin/webpack	Removed webpack wrapper.
bin/eslint	Removed eslint wrapper.
bin/bump-version	Removed version bump helper script.
README.md	Updated dev/build instructions for flakes + npm + Vite.
.ignore	Removed legacy ignore file related to old Nix workflow.
.github/workflows/pages.yml	Added GitHub Pages deployment workflow using `nix build`.
.eslintrc.js	Removed legacy eslint config.
.eslintrc.cjs	Added updated eslint config compatible with module mode.
.eslintignore	Added eslint ignore rules for build artifacts and result dirs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T13:10:44Z

src/mixins/eventable.js

+      removeEventListener(type, listener) {
+        if (!this[`_${type}_listeners`]) {
+          this[`_${type}_listeners`] = {};
+        }
+        const table = this[`_${type}_listeners`];

-				pull(table, listener);
-			},
-			sendEvent(type, ...params) {
-				if (!this[`_${type}_listeners`]) {
-					this[`_${type}_listeners`] = [];
-				}
-				const table = this[`_${type}_listeners`];
+        pull(table, listener);


removeEventListener initializes the listener table to {} when it doesn't exist, but the rest of this mixin treats the table as an array (push/pull/forEach). Initializing to an object can lead to type errors if removeEventListener is called before any listeners are added. Initialize it to [] (or return early when missing) to keep types consistent.

Copilot · 2026-03-04T13:10:44Z

src/index.html

+
+<head>
+	<meta name="theme-color" content="#AFFFFF" />
+	<meta name="viewport" content="width=devide-width, initial-scale=1" />


Viewport meta has a typo: width=devide-width should be width=device-width, otherwise mobile browsers may ignore the intended viewport sizing.

Suggested change

<meta name="viewport" content="width=devide-width, initial-scale=1" />

<meta name="viewport" content="width=device-width, initial-scale=1" />

src/styles/index.css

Copilot · 2026-03-04T13:10:45Z

src/styles/index.css

+.bsod {
+  background: var(--color-borg);
+  color: var(--color-bg);
+  white-space: pre;
+}


In dark mode, .bsod sets color: var(--color-bg) (dark text on a green background). This differs from the previous behavior (white text) and can reduce readability. Use a high-contrast foreground for the BSOD text (e.g. a dedicated variable or var(--color-text)).

Copilot · 2026-03-04T13:10:45Z

src/listener.js

-			this.logger(cleaned, "stomp");
-		}
-	}
+    this.client.debug = (str) => this.debug_callback(str);


@stomp/stompjs v7 Client will not connect until client.activate() is called. The current constructor configures the client but never activates it, so onConnect/subscribe will never run and logs won't stream. Activate the client after setting debug (and consider handling reconnect/cleanup).

Suggested change

this.client.debug = (str) => this.debug_callback(str);

this.client.debug = (str) => this.debug_callback(str);

// Activate the STOMP client so that it actually connects

this.client.activate();

not my code

Copilot · 2026-03-04T13:10:46Z

src/index.html

+			</div>
+		</div>
+	</div>
+	<script src="/index.js" type="module"></script>


<script src="/index.js"> is an absolute path. With GitHub Pages (and base: "./" in vite.config.js), this can break when the site is served from a subpath (e.g. /<repo>/). Prefer a relative module script URL (e.g. index.js or ./index.js) so it works consistently across Pages/custom domains.

Suggested change

<script src="/index.js" type="module"></script>

<script src="./index.js" type="module"></script>

Copilot · 2026-03-04T13:10:46Z

src/lib/bsod.js

-`Hmmm, this is embarassing...
+const bsod = function (msg = "Something happened.") {
+  const body = window.document.body;
+  body.innerText = `Hmmm, this is embarassing...


The BSOD message text contains a typo: "embarassing" should be "embarrassing".

Suggested change

body.innerText = `Hmmm, this is embarassing...

body.innerText = `Hmmm, this is embarrassing...

Copilot · 2026-03-04T13:10:46Z

src/state.js

+  handle_popstate(e) {
+    console.log(e);
+    const { state } = e;


handle_popstate logs the entire popstate event to the console on every history navigation. This is noisy in production and can leak state details into logs; consider removing it or gating behind the existing debug flag.

Copilot · 2026-03-04T13:10:47Z

src/listener.js

-	}
+  disconnect() {
+    this.logger("Disconnecting...", "ofborg");
+    this.client.disconnect(() => this.logger("Disconnected.", "ofborg"));


Client in @stomp/stompjs v7 uses deactivate() (and optionally forceDisconnect()), not the older disconnect(cb) API. As written, disconnect() is likely a runtime error or a no-op depending on the library shim. Update this method to use the v7 lifecycle APIs and ensure any active subscription is unsubscribed before disconnecting.

Suggested change

this.client.disconnect(() => this.logger("Disconnected.", "ofborg"));

if (this.subscription) {

this.subscription.unsubscribe();

this.subscription = null;

}

this.client

.deactivate()

.then(() => {

this.logger("Disconnected.", "ofborg");

})

.catch(() => {

// Even if deactivation fails, we attempted to disconnect.

this.logger("Disconnected.", "ofborg");

});

zebreus mentioned this pull request Feb 10, 2024

Add support for darkmode #27

Closed

zebreus added 15 commits March 4, 2026 12:49

Move colors to css variables

fe7aa2d

Unify similar colors

ffbe889

Adjust css color descriptions

8eb72d6

Add support for prefers-color-scheme: dark

6247f53

Use css variables

b805d94

Change bundler to vite

c3a5968

Switch to flakes and a modern version of node

bda4144

Add github pages deployment

71fc1e7

Adjust vite to use relative resource paths

64c0b24

Add prettier

8c454c8

Format all files using prettier

fa6f08d

Update eslint

a76304d

Add eslint script to package.json

c14990e

Use normal css instead of less

7e6414c

Use dark mode if no preferred color scheme is set

262efb2

Copilot AI review requested due to automatic review settings March 4, 2026 13:03

zebreus force-pushed the modernize branch from 49322cc to 262efb2 Compare March 4, 2026 13:03

Copilot started reviewing on behalf of zebreus March 4, 2026 13:04 View session

Update github action

40da5e0

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Modernize ofborg-viewer#28

Modernize ofborg-viewer#28
zebreus wants to merge 16 commits intoNixOS:masterfrom
zebreus:modernize

zebreus commented Feb 10, 2024

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

zebreus Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	<meta name="viewport" content="width=devide-width, initial-scale=1" />
	<meta name="viewport" content="width=device-width, initial-scale=1" />

	<script src="/index.js" type="module"></script>
	<script src="./index.js" type="module"></script>

	body.innerText = `Hmmm, this is embarassing...
	body.innerText = `Hmmm, this is embarrassing...

-    this.client.disconnect(() => this.logger("Disconnected.", "ofborg"));
+    if (this.subscription) {
+      this.subscription.unsubscribe();
+      this.subscription = null;
+    }
+    this.client
+      .deactivate()
+      .then(() => {
+        this.logger("Disconnected.", "ofborg");
+      })
+      .catch(() => {
+        // Even if deactivation fails, we attempted to disconnect.
+        this.logger("Disconnected.", "ofborg");
+      });

Uh oh!

Conversation

zebreus commented Feb 10, 2024

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

zebreus Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants