Increase coverage for frontend components and functions

[leekahung]

…ontend; Simplified/refactored existing tests; Removing false positive tests

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Infrastructure
• Maintenance

Description

This PR increases coverage for frontend components and functions , simplifies/refactors unit tests, and removes false positive tests

Related Tickets & Documents

• Related Issue #
• Closes #

QA Instructions, Screenshots, Recordings

Please replace this line with instructions on how to test your changes, a note on the devices and browsers this has been tested on, as well as any relevant images for UI changes.

Added/updated tests?

• Yes
• No, and this is why: please replace this line with details on why tests have not been included
• I need help with writing tests

Documentation

• If this PR changes the system architecture, Architecture.md has been updated

[optional] Are there any post deployment tasks we need to perform?

[leekahung]

@claude pr-check

[claude]

Claude finished @leekahung's task in 3m 8s —— View job

---

PR Review: Increase frontend coverage

CI: ✅ All checks passing

Overall this is a solid batch of tests. The new files fill real coverage gaps, the render-helper pattern is used consistently, and the refactored tests are cleaner than what they replaced. A few things worth looking at:

---

Bug in source code not caught by tests

FeedbackModal.tsx:67-70 has a missing return after the early-close branch:

onClick={() => {
  if (feedback.trim() === "") handleModalClose();  // no return!
  setStatus("sending");   // always runs, even when feedback is empty
  setTimeout(() => { ... }, 1000);
}}

When feedback is empty, handleModalClose() is called (which calls setOpenFeedback(false) and resets state), but execution falls through to setStatus("sending"), which then renders "Feedback Sent!" and schedules a setTimeout. In the real app the component unmounts immediately, masking the bug. In the test, setOpenFeedback is mocked so the component stays alive — meaning "Feedback Sent!" does briefly render even on empty submission.

The test "closes immediately when Send is clicked with empty feedback" only asserts that setOpenFeedback(false) was called — it doesn't assert that "Feedback Sent!" is not shown, so the bug goes undetected.

Consider adding: Fix this →

// frontend/src/tests/components/FeedbackModal.test.tsx
it("closes immediately when Send is clicked with empty feedback", () => {
  const setOpenFeedback = vi.fn();
  render(<FeedbackModal messages={messages} setOpenFeedback={setOpenFeedback} />);
  fireEvent.click(screen.getByRole("button", { name: "Send" }));
  expect(setOpenFeedback).toHaveBeenCalledWith(false);
  // Add: expect(screen.queryByText("Feedback Sent!")).not.toBeInTheDocument();
});

---

Minor issues

InputField.test.tsx:14 — createRef<HTMLTextAreaElement | null>(): the | null union is redundant. React's RefObject<T>.current is already T | null. This should be createRef<HTMLTextAreaElement>().

FeedbackModal.test.tsx — vi.useRealTimers() is called unconditionally in afterEach, but only one test calls vi.useFakeTimers(). This is harmless but slightly misleading — consider scoping the fake-timer setup and teardown to just the one test that needs it.

HousingContext.test.tsx:16 — handleHousingLocation?.({...}) uses optional chaining, which would silently no-op if the function were missing. Since the context always provides it, this should be a direct call. The optional chaining would mask a regression if the context API changed.

MessageWindow.test.tsx:14 — beforeAll is declared outside the describe block. It works in Vitest but convention (and isolation) favors placing it inside the describe.

---

What's good

• The render-helper pattern (renderInputField, renderMessageWindow, renderDialog) is clean and consistently applied — good reduction of boilerplate.
• deserializeAiMessage unit tests are thorough and test edge cases (reasoning-only, mixed chunks, plain text fallback) well.
• useIsMobile tests correctly verify both initial state and the resize event flow, including cleanup.
• SafeMarkdown tests check security-relevant link attributes (target="_blank", rel="noopener noreferrer") — good to have these explicit.
• Removing the false-positive tests from formHelper.test.ts (the 24 deleted lines) is the right call.

[leekahung]

@claude pr-check

[claude]

Claude finished @leekahung's task in 1m 34s —— View job

---

PR Review: Increase frontend coverage

CI: ✅ All checks passing

All feedback from the previous review has been addressed. Here's a quick summary of what was fixed and one remaining nit:

---

Fixed since last review

• FeedbackModal.tsx:68 — return handleModalClose() is now correct; the source bug (falling through to setStatus("sending") on empty feedback) is fixed.
• FeedbackModal.test.tsx — The empty-feedback test now asserts expect(screen.queryByText("Feedback Sent!")).not.toBeInTheDocument(), catching the bug if it regresses.
• FeedbackModal.test.tsx — vi.useRealTimers() is now scoped inside the one test that uses fake timers rather than in afterEach.
• InputField.test.tsx — Redundant | null removed from createRef<HTMLTextAreaElement>().
• HousingContext.test.tsx — Optional chaining on handleHousingLocation?.() replaced with a proper test component; the context is exercised via a real button click.
• MessageWindow.test.tsx — beforeAll is now inside the describe block.

---

One remaining nit

LetterGenerationDialog.test.tsx:8 — Same pattern as the InputField issue that was just fixed: createRef<HTMLDialogElement | null>() has a redundant | null. React's RefObject<T>.current is already T | null, so this should be createRef<HTMLDialogElement>(). Fix this →

---

What's good

The PR is in solid shape. The new tests fill real coverage gaps with a clean, consistent pattern. deserializeAiMessage tests are thorough, useIsMobile correctly tests the resize event flow and cleanup, and SafeMarkdown tests verify security-relevant link attributes. The false-positive removal from formHelper.test.ts was the right call.