fix: address remaining copilot review feedback

kartikrautan · kartikrautan · commit c9c20d10dbb9 · 2026-03-07T17:41:44.000+05:30
Signed-off-by: kartik &lt;kartikrautan0@gmail.com&gt;
diff --git a/dco_log.txt b/dco_log.txt
diff --git a/dco_log_utf8.txt b/dco_log_utf8.txt
@@ -0,0 +1,84 @@
+﻿c10cf29 | kartik <kartikrautan0@gmail.com> | Merge branch 'differentLanguageSupport' of https://github.com/kartikrautan/template-playground into differentLanguageSupport | Signed-off-by: kartik <kartikrautan0@gmail.com>
+
+d6199d2 | kartik <kartikrautan0@gmail.com> | Updation on copilot reviews | 
+9a71e91 | Kartik Rautan <kartikrautan0@gmail.com> | Merge branch 'main' into differentLanguageSupport | 
+76717a7 | Harshit Kumar <10harshitkumar@gmail.com> | feat: Implement Error Boundary for better error handling (#734) | * feat: add ErrorBoundary to prevent app crashes
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* refactor: address code review feedback
+
+- Add type='button' to prevent unintended form submission
+- Hide error details in production for security (only show in dev mode)
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* test(ErrorBoundary): add comprehensive test coverage for error boundary component
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* feat(ErrorBoundary): add theme-aware colors and component stack to error details
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* fix(ErrorBoundary): address Copilot review suggestions for test quality
+
+- Add showDevDetails prop for testable dev mode control
+
+- Fix window.location.reload mock to avoid test pollution
+
+- Correct dark mode color to #121212 (matches store)
+
+- Remove import.meta.env.DEV mutations (read-only property)
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* fix(ErrorBoundary): resolve TypeScript error in test
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+* fix(ErrorBoundary): implement 3 additional Copilot suggestions
+
+- Properly type consoleErrorSpy as MockInstance (no eslint-disable)
+
+- Add async/await for component stack test to handle state update timing
+
+- Capture and restore Zustand state in theme test to prevent order-dependency
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+
+---------
+
+Signed-off-by: Harshit Kumar <10harshitkumar@gmail.com>
+Co-authored-by: Matt Roberts <7544022+mttrbrts@users.noreply.github.com>
+7b51f8a | kartik <kartikrautan0@gmail.com> | Merge remote-tracking branch 'upstream/main' into differentLanguageSupport | Signed-off-by: kartik <kartikrautan0@gmail.com>
+
+# Conflicts:
+#	package-lock.json
+
+b8ead94 | kartik <kartikrautan0@gmail.com> | Different Language Support | Signed-off-by: kartik <kartikrautan0@gmail.com>
+
+cb2e662 | Vaggelis Argyropoulos <69078031+Vaggelis-Arg@users.noreply.github.com> | fix(AIConfigPopup): UX: validate model selection and allow API key visibility (#744) | Signed-off-by: Evangelos Argyropoulos <vag.argyropoulos@gmail.com>
+1272f54 | Majoju Vaishnavi <majojuvaishnavi@gmail.com> | feat(playground): improve default agreement sample (#702) | * feat(playground): improve default agreement sample to better reflect legal use case
+
+Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
+
+* Update Service Agreement template
+
+Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
+
+---------
+
+Signed-off-by: Vaishnavi <majojuvaishnavi@gmail.com>
+Co-authored-by: Matt Roberts <7544022+mttrbrts@users.noreply.github.com>
+ddf33c2 | Kartik Rautan <kartikrautan0@gmail.com> | fix: Replace plaintext API key storage with hardware-backed WebAuthn PRF + AES-GCM encryption (#762) | * Replaced plaintext storage with WebAuthn PRF encryption
+
+Signed-off-by: kartik <kartikrautan0@gmail.com>
+
+* Updation on copilot reviews
+
+Signed-off-by: kartik <kartikrautan0@gmail.com>
+
+---------
+
+Signed-off-by: kartik <kartikrautan0@gmail.com>
diff --git a/pr_diff.txt b/pr_diff.txt
diff --git a/src/components/Navbar.tsx b/src/components/Navbar.tsx
@@ -213,7 +213,8 @@ function Navbar() {
     void i18n.changeLanguage(lng);
   };
 
-  const currentLang = LANGUAGES.find(l => l.code === i18n.language) || LANGUAGES[0];
+  const currentLanguageCode = i18n.resolvedLanguage || (i18n.language ? i18n.language.split('-')[0] : 'en');
+  const currentLang = LANGUAGES.find(l => l.code === currentLanguageCode) || LANGUAGES[0];
 
   const props = useSpring({
     loop: true,
@@ -231,7 +232,7 @@ function Navbar() {
         <MenuItem
           key={lang.code}
           onClick={() => changeLanguage(lang.code)}
-          className={i18n.language === lang.code ? 'bg-gray-100 dark:bg-gray-700 font-semibold' : ''}
+          className={currentLanguageCode === lang.code ? 'bg-gray-100 dark:bg-gray-700 font-semibold' : ''}
         >
           <span>{lang.flag}</span>
           <span>{lang.label}</span>
diff --git a/src/tests/components/Footer.test.tsx b/src/tests/components/Footer.test.tsx
@@ -2,10 +2,7 @@ import { render } from "@testing-library/react";
 import { describe, it, expect, vi, beforeAll } from "vitest";
 import Footer from "../../components/Footer";
 
-vi.mock('react-i18next', async () => {
-  const { reactI18nextMock } = await import("../../utils/testing/i18nMock");
-  return reactI18nextMock;
-});
+
 
 vi.mock("../../public/logo.png", () => ({
   default: "logo.png",
diff --git a/src/tests/components/Navbar.test.tsx b/src/tests/components/Navbar.test.tsx
@@ -3,11 +3,13 @@ import "@testing-library/jest-dom";
 import Navbar from "../../components/Navbar";
 import { MemoryRouter } from "react-router-dom";
 import { vi, describe, it, expect, afterEach } from "vitest";
-import { mockChangeLanguage } from "../../utils/testing/i18nMock";
 
-vi.mock("react-i18next", async () => {
-  const { reactI18nextMock } = await import("../../utils/testing/i18nMock");
+const mockChangeLanguage = vi.fn();
+
+vi.mock("react-i18next", async (importOriginal) => {
+  const actual = await importOriginal<typeof import('react-i18next')>();
   return {
+    ...actual,
     useTranslation: () => ({
       t: (key: string) => {
         const map: Record<string, string> = {
@@ -25,7 +27,12 @@ vi.mock("react-i18next", async () => {
         };
         return map[key] || key;
       },
-      i18n: reactI18nextMock.useTranslation().i18n,
+      i18n: {
+        get language() {
+          return localStorage.getItem("i18nextLng") || "en";
+        },
+        changeLanguage: mockChangeLanguage,
+      },
     }),
   };
 });
diff --git a/src/tests/components/PlaygroundSidebar.test.tsx b/src/tests/components/PlaygroundSidebar.test.tsx
@@ -4,9 +4,13 @@ import PlaygroundSidebar from "../../components/PlaygroundSidebar";
 import { vi } from "vitest";
 
 
-vi.mock('react-i18next', async () => {
-    const { reactI18nextMock } = await import("../../utils/testing/i18nMock");
+vi.mock('react-i18next', async (importOriginal) => {
+    const actual = await importOriginal<typeof import('react-i18next')>();
+    // The global mock in setup.ts doesn't export its inner reactI18nextMock.
+    // Instead, we just provide the specific `t` we need and a basic `i18n` object
+    // since vitest hoists this. We can use the global mockChangeLanguage.
     return {
+        ...actual,
         useTranslation: () => ({
             t: (key: string) => {
                 const map: Record<string, string> = {
@@ -21,7 +25,12 @@ vi.mock('react-i18next', async () => {
                 };
                 return map[key] || key;
             },
-            i18n: reactI18nextMock.useTranslation().i18n,
+            i18n: {
+                get language() {
+                    return localStorage.getItem("i18nextLng") || "en";
+                },
+                changeLanguage: vi.fn(), // We can just mock this per file if needed, or import the global one if it wasn't hoisted
+            },
         })
     };
 });
diff --git a/src/tests/components/SettingsModal.test.tsx b/src/tests/components/SettingsModal.test.tsx
@@ -2,9 +2,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { render, screen } from '@testing-library/react';
 import SettingsModal from '../../components/SettingsModal';
 
-vi.mock('react-i18next', async () => {
-  const { reactI18nextMock } = await import('../../utils/testing/i18nMock');
+vi.mock('react-i18next', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('react-i18next')>();
   return {
+    ...actual,
     useTranslation: () => ({
       t: (key: string) => {
         const map: Record<string, string> = {
@@ -16,7 +17,12 @@ vi.mock('react-i18next', async () => {
         };
         return map[key] || key;
       },
-      i18n: reactI18nextMock.useTranslation().i18n,
+      i18n: {
+        get language() {
+          return localStorage.getItem("i18nextLng") || "en";
+        },
+        changeLanguage: vi.fn(),
+      },
     })
   };
 });
diff --git a/src/utils/testing/i18nMock.ts b/src/utils/testing/i18nMock.ts
diff --git a/src/utils/testing/setup.ts b/src/utils/testing/setup.ts
@@ -1,4 +1,4 @@
-import { expect, afterEach } from "vitest";
+import { expect, afterEach, vi } from "vitest";
 import { cleanup } from "@testing-library/react";
 import * as matchers from "@testing-library/jest-dom/matchers";
 
@@ -60,32 +60,47 @@ HTMLCanvasElement.prototype.getContext = ((originalGetContext) => {
     if (contextId === '2d') {
       return {
         fillStyle: '',
-        fillRect: () => {},
-        clearRect: () => {},
+        fillRect: () => { },
+        clearRect: () => { },
         getImageData: () => ({ data: [] }),
-        putImageData: () => {},
+        putImageData: () => { },
         createImageData: () => ({ data: [] }),
-        setTransform: () => {},
-        drawImage: () => {},
-        save: () => {},
-        restore: () => {},
-        beginPath: () => {},
-        moveTo: () => {},
-        lineTo: () => {},
-        closePath: () => {},
-        stroke: () => {},
-        fill: () => {},
-        translate: () => {},
-        scale: () => {},
-        rotate: () => {},
-        arc: () => {},
+        setTransform: () => { },
+        drawImage: () => { },
+        save: () => { },
+        restore: () => { },
+        beginPath: () => { },
+        moveTo: () => { },
+        lineTo: () => { },
+        closePath: () => { },
+        stroke: () => { },
+        fill: () => { },
+        translate: () => { },
+        scale: () => { },
+        rotate: () => { },
+        arc: () => { },
         measureText: () => ({ width: 0 }),
-        transform: () => {},
-        rect: () => {},
-        clip: () => {},
+        transform: () => { },
+        rect: () => { },
+        clip: () => { },
         canvas: this,
       } as unknown as CanvasRenderingContext2D;
     }
     return originalGetContext.call(this, contextId as any, options);
   };
 })(HTMLCanvasElement.prototype.getContext);
+
+// Global mock for react-i18next
+export const mockChangeLanguage = vi.fn();
+
+vi.mock("react-i18next", () => ({
+  useTranslation: () => ({
+    t: (key: string) => key,
+    i18n: {
+      get language() {
+        return localStorage.getItem("i18nextLng") || "en";
+      },
+      changeLanguage: mockChangeLanguage,
+    },
+  }),
+}));
