This revision should only contain the NotFound.vue page and its catch-all route — that's the task. Please strip out the unrelated changes (navbar logo rework, SubscribeForm restyling, logo.svg replacement, package-lock.json removals) into their own separate revisions. See inline comments for specific fixes needed on indentation and missing newlines.

Also, please add me as a reviewer when you create a revision (run arc diff --nolint --reviewers ieli works I believe, otherwise just add me manually in Phabricator). That way I get notified and can review sooner.

frontend/package-lock.json
695–4392 ↗	(On Diff #10293)	This removes @parcel/watcher, sass, chokidar, detect-libc, immutable, node-addon-api, and readdirp. These are likely side-effects of running npm install on a different OS/environment. ⚠️ Please revert this file or explain why these removals are intentional.
frontend/src/assets/logo.svg
1–235	This is a .svg file but it now contains a full Vue Single File Component (<template>, <script setup>, ref, computed, requestAnimationFrame…). A .svg file won't be processed by Vite/Vue as a component, this needs to be a .vue file if it's meant to be a component. ⚠️ Also, this is unrelated to the NotFound task, please move it to its own revision. Missing trailing newline too.
frontend/src/components/AppNavbar.vue
19–95	This entire logo feature (custom logo via config + Vue logo fallback) is unrelated to the NotFound task. ⚠️ Please split into a separate revision. Also: the Vue.js framework logo should not be the fallback, end users shouldn't see a framework logo. Either show nothing or use a ServPulse icon.
62	The interpolation and </span> closing tag lost their indentation. The <span> opens at 10 spaces, so the content should be at 12 and </span> at 10. Right now they're at 4 and 2, looks like a copy-paste alignment issue.
frontend/src/components/SubscribeForm.vue
37–125	These are cosmetic/layout changes (centering, max-w-md, p-5→p-6, full-width buttons) unrelated to the NotFound page. ⚠️ Please move to a separate revision.
124	⚠️ Missing trailing newline at end of file. POSIX standard requires it, some tools (diff, cat, shell) may misbehave without it, and our other .vue files all have one. {F24438978}
frontend/src/router/index.js
35–37	Indentation is inconsistent with the rest of the routes array. Should use 6 spaces (2 for the array + 4 for the object), matching the pattern of all other routes above.
38	Inconsistent end of bracket.
frontend/src/views/NotFound.vue
26	⚠️ Missing trailing newline at end of file.

This revision now requires changes to proceed.Fri, Feb 20, 17:55

Chenani-MohamedAmine edited the summary of this revision. (Show Details)Tue, Feb 24, 12:39

Chenani-MohamedAmine mentioned this in D3977: Add 404 error page and catch-all route.Tue, Feb 24, 12:52

I've reviewed your changes. I noticed you created a new differential (D3977) instead of updating this one (D3971) for the same task.

When making revisions based on review feedback, you should amend the existing differential rather than creating a new one. This keeps the review history in one place and avoids duplicate diff IDs.

For future reference:

Use arc diff to update the existing differential with your changes
Only create a new differential if it's a genuinely separate/unrelated task

Creating separate differentials for the same task is the fastest and most reliable way to chaos and loss of control over a project. It fragments the review history, makes it impossible to track what was actually merged, and creates confusion about which version is canonical. In team environments, this quickly becomes unmanageable.

ieli mentioned this in T2254: Design ServPulse logo.Tue, Feb 24, 22:12

Reverted accidental OS-specific changes in package-lock.json

Harbormaster completed remote builds in B6368: Diff 10317.Wed, Feb 25, 00:06

Stripped unrelated logo/navbar/package-lock files and fixed trailing newlines

Harbormaster completed remote builds in B6369: Diff 10318.Wed, Feb 25, 00:13

Chenani-MohamedAmine edited the summary of this revision. (Show Details)Wed, Feb 25, 00:14

ieli added inline comments.Wed, Feb 25, 00:36

frontend/src/router/index.js
35–37	Indentation issues still present when I pull your diff locally.
44–66	Indentation issues still present when I pull your diff locally.
46–54	Indentation issues still present when I pull your diff locally.
59	The `export default router` appears twice. This causes a syntax error.
frontend/src/views/__tests__/NotFound.test.js
6–16 ↗	(On Diff #10318)	Indentation issues still present

ieli added inline comments.Wed, Feb 25, 00:41

frontend/src/views/__tests__/NotFound.test.js
17 ↗	(On Diff #10318)	Consider also testing the RouterLink to verify it navigates to `/`. You can stub RouterLink with a custom component that exposes the `to` prop.