refactor: remove unnecessary comments and code

Author: awesthouse

web/src/app/auth/SessionExpiredDialog/SessionExpiredDialog.tsx

@@ -1,18 +1,11 @@
 /**
- * Modal dialog shown when `httpClient` latches `sessionExpiredStore`
+ * Non-dismissable Modal dialog shown when `httpClient` latches `sessionExpiredStore`
  * on a 401. Pure presentation: state and the popup handshake live in
  * `useReauthFlow` (under `platform/auth`).
- *
- * Built on the native `<dialog>` element via `showModal()`, which gives
- * us focus trap, top-layer rendering, and focus restoration for free.
- * Esc is intercepted (`cancel` event) to keep the dialog modal-required
- * — dismissing only delays the next 401, which immediately re-opens
- * the dialog. The only escape is "Sign in".
  */
 
-import { Button, Card, Typography } from '@equinor/eds-core-react'
+import { Button, Dialog as EDSDialog, Typography } from '@equinor/eds-core-react'
 import { useQueryClient } from '@tanstack/react-query'
-import { type SyntheticEvent, useEffect, useRef } from 'react'
 import { useReauthFlow } from '@/shared/platform/auth'
 
 const TITLE_ID = 'session-expired-title'
@@ -21,58 +14,29 @@ const DESC_ID = 'session-expired-description'
 export const SessionExpiredDialog = () => {
   const queryClient = useQueryClient()
   const { open, signingIn, requestSignIn } = useReauthFlow(queryClient)
-  return open ? <Dialog signingIn={signingIn} onSignIn={requestSignIn} /> : null
-}
-
-interface DialogProps {
-  signingIn: boolean
-  onSignIn: () => void
-}
-
-const Dialog = ({ signingIn, onSignIn }: DialogProps) => {
-  const ref = useRef<HTMLDialogElement>(null)
-
-  useEffect(() => {
-    const dialog = ref.current
-    if (!dialog) return
-    dialog.showModal()
-    return () => {
-      if (dialog.open) dialog.close()
-    }
-  }, [])
-
-  // Block Esc-to-dismiss; see file header.
-  const blockCancel = (e: SyntheticEvent<HTMLDialogElement>) => e.preventDefault()
-
-  return (
-    <dialog
-      ref={ref}
+  return open ? (
+    <EDSDialog
       aria-labelledby={TITLE_ID}
       aria-describedby={DESC_ID}
-      onCancel={blockCancel}
-      style={{
-        border: 'none',
-        padding: 0,
-        background: 'transparent',
-        maxWidth: '24rem',
-        margin: 'auto',
-      }}
+      isDismissable={false}
+      open
+      style={{ width: '100vw', maxWidth: 420 }}
     >
-      <Card style={{ padding: '1.5rem', display: 'grid', gap: '1rem' }}>
-        <Typography id={TITLE_ID} variant="h5">
-          Session expired
-        </Typography>
+      <EDSDialog.Header>
+        <EDSDialog.Title id={TITLE_ID}>Session expired</EDSDialog.Title>
+      </EDSDialog.Header>
+      <EDSDialog.Content>
         <Typography id={DESC_ID} variant="body_short">
           {signingIn
             ? 'Complete sign-in in the popup window. This dialog will close automatically when you’re done.'
             : 'Your session has expired. Sign in again in a popup — this page and your work will stay as they are.'}
         </Typography>
-        <div style={{ display: 'flex', justifyContent: 'flex-end' }}>
-          <Button onClick={onSignIn} disabled={signingIn}>
+        <div style={{ display: 'flex', justifyContent: 'flex-end', marginTop: '1rem' }}>
+          <Button onClick={requestSignIn} disabled={signingIn}>
             {signingIn ? 'Waiting for sign-in…' : 'Sign in'}
           </Button>
         </div>
-      </Card>
-    </dialog>
-  )
+      </EDSDialog.Content>
+    </EDSDialog>
+  ) : null
 }

web/src/app/bootstrap/Providers.tsx

@@ -1,22 +1,23 @@
-import { QueryClientProvider } from '@tanstack/react-query'
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
 import { ReactQueryDevtools } from '@tanstack/react-query-devtools'
 import { SessionExpiredDialog } from '@/app/auth/SessionExpiredDialog/SessionExpiredDialog'
 import { ApplicationError } from '@/app/bootstrap/ApplicationError'
-import { TelemetryErrorBoundary, TelemetryProvider } from '@/shared/platform/telemetry'
+import { Telemetry, TelemetryErrorBoundary, TelemetryProvider } from '@/shared/platform/telemetry'
 import { ToastContainer } from '@/shared/platform/toast'
-import type { ProvidersProps } from './Providers.types'
 
-// No `<AppInsightsContext.Provider>`: page-views use `enableAutoRouteTracking`
-// and nothing currently calls `useAppInsightsContext`.
+export interface ProvidersProps {
+  telemetry: Telemetry
+  queryClient: QueryClient
+  children: React.ReactNode
+}
+
 export const Providers = ({ telemetry, queryClient, children }: ProvidersProps) => (
   <TelemetryProvider telemetry={telemetry}>
     <TelemetryErrorBoundary telemetry={telemetry} fallback={ApplicationError}>
       <QueryClientProvider client={queryClient}>
         <SessionExpiredDialog />
         {children}
         <ToastContainer />
-        {/* `import.meta.env.DEV` is statically replaced by Vite, so the
-            devtools import is tree-shaken from production builds. */}
         {import.meta.env.DEV && <ReactQueryDevtools initialIsOpen={false} />}
       </QueryClientProvider>
     </TelemetryErrorBoundary>

web/src/app/bootstrap/Providers.types.ts

@@ -1,4 +1,4 @@
-@reference "tailwindcss";
+@reference "../../../../styles/index.css";
 
 .color-schema-radio {
     @apply flex flex-1 flex-col items-center text-center px-4 py-2 border border-border rounded cursor-pointer;

web/src/app/layout/Header/components/ColorSchemaToggle/ColorSchemaToggle.css

@@ -4,7 +4,7 @@ import { ENV } from '@/config/env'
 import type { CommitInfo } from './VersionText.types'
 import { EMPTY_COMMIT_INFO, parseVersionFile } from './VersionText.utils'
 
-const useCommitInfo = () => {
+export const VersionText = () => {
   const [commitInfo, setCommitInfo] = useState<CommitInfo>(EMPTY_COMMIT_INFO)
 
   useEffect(() => {
@@ -17,11 +17,6 @@ const useCommitInfo = () => {
       .catch(() => setCommitInfo(EMPTY_COMMIT_INFO))
   }, [])
 
-  return commitInfo
-}
-
-export const VersionText = () => {
-  const commitInfo = useCommitInfo()
   const label = commitInfo.refs || commitInfo.hash || 'unknown'
 
   return (

web/src/app/layout/VersionText/VersionText.tsx

@@ -12,9 +12,8 @@ export interface RouterContext {
 }
 
 export const Route = createRootRouteWithContext<RouterContext>()({
-  // Resolve the user once before any component mounts so suspense reads
-  // are synchronous, and bridge identity to telemetry in the same step.
   beforeLoad: async ({ context: { queryClient, telemetry } }) => {
+    // Resolve the user once before any component mounts
     const user = await queryClient.ensureQueryData(userQuery())
     telemetry.setUser(user.id)
   },

web/src/app/routes/__root.tsx

@@ -1,31 +1,21 @@
 import { createFileRoute } from '@tanstack/react-router'
 import { z } from 'zod'
 import { hasPermissionCheck } from '@/config/accessControl'
-import { todoListQuery, TodosPage } from '@/features/todos'
+import { TodosPage, todoListQuery } from '@/features/todos'
 import { userQuery } from '@/shared/platform/auth'
 
-// `?status=` is the only piece of URL state owned by this page; the
-// schema doubles as the runtime validator and the type fed to
-// `useSearch`. Defaulting to `'all'` keeps the URL clean — TanStack
-// Router omits defaulted keys from the rendered query string.
+// Validate the `status` search param and provide a default value of 'all' if it's not present.
 const searchSchema = z.object({
   status: z.enum(['all', 'active', 'done']).default('all'),
 })
 
 export const Route = createFileRoute('/todos')({
-  // Gate the route on `todos:read`. The root route already primed
-  // `userQuery`, so this is a cheap cache hit; calling it here keeps
-  // the route self-contained and matches the pattern documented in
-  // `shared/platform/access-control/README.md`. A 403 `Response` is
-  // caught by `RouteErrorBoundary`.
   beforeLoad: async ({ context: { queryClient } }) => {
     const user = await queryClient.ensureQueryData(userQuery())
     if (hasPermissionCheck(user, 'todos', 'read') !== true) {
       throw new Response(null, { status: 403 })
     }
   },
-  // Prime the todo list cache so `useTodos` (useSuspenseQuery) gets an
-  // instant cache hit on first paint instead of suspending.
   loader: ({ context: { queryClient } }) => queryClient.ensureQueryData(todoListQuery()),
   component: TodosPage,
   validateSearch: searchSchema,

web/src/app/routes/todos.tsx

@@ -130,6 +130,8 @@
 		outline: 2px dashed var(--border-focus);
 		outline-offset: 2px;
 	}
+	/* Fix tailwind reset on EDS dialog */
+	dialog { margin: auto }
 	dialog::backdrop {
 		background: rgba(0, 0, 0, 0.4);
 	}

web/src/app/styles/index.css

@@ -9,47 +9,29 @@
  *   - `hasPermissionCheck(user, resource, action, data?)` — the bound
  *     checker. Returns `boolean | 'loading'`. Used from route
  *     `beforeLoad` (throws a 403 `Response`).
- *   - `isAllowed(user, resource, action, data?)` — strict-boolean
- *     variant for components and tests; `'loading'` collapses to
- *     `false`.
- *
- * Leaf-shaped on purpose (no React, no auth imports beyond the
- * type-only `CurrentUser`) so it can be safely imported from anywhere
- * including `userQuery`.
- *
- * To add a role: extend the `Role` union, give it an entry in `ROLES`,
- * and make sure the backend (`/whoami`) emits the matching string in
- * its `roles` array. Unknown role strings are silently ignored.
+ *   - `isAllowed(user, resource, action, data?)` — Strict-boolean variant of {@link hasPermissionCheck}: `
+ *     'loading'` collapses to `false`. Prefer this in components and tests; loaders
+ *     should keep using `hasPermissionCheck` so they can distinguish`'loading'` from a real deny if needed.
  */
 
 import type { Todo } from '@/features/todos'
 import { createHasPermission, createIsAllowed, type RolesWithPermissions } from '@/shared/platform/access-control'
 import type { CurrentUser } from '@/shared/platform/auth'
 
-// ---------------------------------------------------------------------------
-// Roles
-// ---------------------------------------------------------------------------
-
+/* Make sure roles stay in sync with the backend's `/whoami`
+ * response as unknown role strings are silently ignored.
+ */
 export type Role = 'admin' | 'editor' | 'viewer'
 
-// ---------------------------------------------------------------------------
-// Permissions
-// ---------------------------------------------------------------------------
-
 export type Permissions = {
   todos: {
     dataType: Todo | null
     action: 'read' | 'create' | 'update' | 'delete'
   }
 }
 
-/** True iff `todo` exists and its `user_id` matches the current user. */
 const isOwnTodo = (user: CurrentUser, todo: Todo | null): boolean => todo != null && todo.user_id === user.id
 
-// ---------------------------------------------------------------------------
-// Roles → permissions table
-// ---------------------------------------------------------------------------
-
 export const ROLES: RolesWithPermissions<Role, Permissions, CurrentUser> = {
   admin: {
     todos: { read: true, create: true, update: true, delete: true },
@@ -58,7 +40,6 @@ export const ROLES: RolesWithPermissions<Role, Permissions, CurrentUser> = {
     todos: {
       read: true,
       create: true,
-      // Editors may only modify todos they own.
       update: isOwnTodo,
       delete: isOwnTodo,
     },
@@ -68,17 +49,5 @@ export const ROLES: RolesWithPermissions<Role, Permissions, CurrentUser> = {
   },
 }
 
-/**
- * Singleton check bound to {@link ROLES}. Loader call sites (`*.route.ts`)
- * call this directly and throw a 403 `Response` — caught by
- * `RouteErrorBoundary` → `<ForbiddenPage />`.
- */
 export const hasPermissionCheck = createHasPermission<Role, Permissions, CurrentUser>(ROLES)
-
-/**
- * Strict-boolean variant of {@link hasPermissionCheck}: `'loading'`
- * collapses to `false`. Prefer this in components and tests; loaders
- * should keep using `hasPermissionCheck` so they can distinguish
- * `'loading'` from a real deny if needed.
- */
 export const isAllowed = createIsAllowed<Permissions, CurrentUser>(hasPermissionCheck)

web/src/config/accessControl.ts

@@ -1,14 +1,3 @@
-// Public surface — for cross-feature imports only (e.g. `src/routes/`).
-// Files inside this feature must use relative paths instead of importing from
-// here; otherwise we get circular imports and the lazy-chunked page bundle
-// pulls in everything reachable from this barrel.
-//
-// Enforced by the `no-restricted-imports` rule in `eslint.config.mts` —
-// outside files cannot deep-import `@/features/<name>/...`.
-//
-// Code-splitting is now handled by the TanStack Router Vite plugin's
-// `autoCodeSplitting` — the chunk boundary is the route file (`routes/index.tsx`),
-// not the import path. Re-exporting `TodosPage` here is therefore safe.
 export { todoListQuery } from './api'
 export type { Todo } from './api/schema'
 export { TodosPage } from './pages/TodosPage/TodosPage'