Homepage

[hermannsalan]

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

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

Description

This is a version of the homepage and an additional form with a tie-in to the submit feedback function.
The form, the helper function, and the hp have separate files. Styling in the homepage is contained to that file, & likewise in the form ~

Related Tickets & Documents

• Related Issue #
• Closes #

QA Instructions, Screenshots, Recordings

I think the form needs to be tested- I haven't had the backend running locally !

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]

Hey Alan, thanks for making the home page! Here are some feedback related to the changes.

[leekahung]

@claude can you perform a PR check as well?

[claude]

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

---

PR Check — Homepage (#305)

• Gather context
• Read changed files
• Review code quality, bugs, and conventions
• Post final review

---

Nice work getting a homepage up! Here are issues I found beyond what @leekahung already caught — prioritized by severity.

---

🐛 Bug: Form submission not prevented

HPFeedbackForm.tsx:35-40 — The button's onClick uses setTimeout but never calls event.preventDefault(). When the form submit event fires, the page reloads before sendHPFeedback ever runs. The feedback is silently lost.

// current — broken
<button type="submit" className="btn-hover" style={styles.formSubmit}
  onClick={() => {
    setTimeout(() => {
      sendHPFeedback(name, subject, feedback);
    }, 1000);
  }}>Submit</button>

This should be restructured: use onSubmit on the <form>, call e.preventDefault(), then call sendHPFeedback directly (no setTimeout needed). Fix this →

---

🐛 Bug: Wrong import paths in HP.tsx

HP.tsx:2-3 — The imports use ../src/shared/... but this file is already inside src/. It accidentally works because ../src/ navigates up to frontend/ and then back into src/, but it's fragile and wrong.

// wrong
import BeaverIcon from '../src/shared/components/BeaverIcon';
import TenantFirstAidLogo from "../src/shared/components/TenantFirstAidLogo";

// correct
import BeaverIcon from './shared/components/BeaverIcon';
import TenantFirstAidLogo from './shared/components/TenantFirstAidLogo';

Fix this →

---

⚠️ Duplicate/conflicting Navbar

frontend/src/shared/components/Navbar.tsx (new) duplicates the existing Navbar/Navbar.tsx directory. App.tsx still imports from Navbar/Navbar, so the new file is unused dead code. It should be removed. If the goal was to add new nav links or update the navbar, changes should go into the existing Navbar/ directory.

---

⚠️ Dead code: duplicate styles in HP.tsx

HP.tsx:455-459 — formSection, contactForm, formInput, formTextarea, and formSubmit are defined in the styles object but also defined identically inside HPFeedbackForm.tsx:4-10. Since the form renders its own styles, the copies in HP.tsx are never used.

---

⚠️ Misplaced helper file

frontend/src/pages/Chat/utils/feedbackHPHelper.ts — This helper is for the homepage, not for Chat. Placing it under Chat/utils/ is misleading. It should either live in shared/utils/ or in a new pages/Home/ directory (consistent with the reviewer's suggestion for a dedicated Home page directory).

---

⚠️ Missing error handling in sendHPFeedback

feedbackHPHelper.ts:49-52 — The fetch call has no error handling. If the API call fails, the user gets no feedback. At minimum, the error should be surfaced to the caller so the form can show a failure state.

---

📝 Typo

HP.tsx:288 — "relevent" → "relevant". Fix this →

---

📝 Empty paragraph in hero section

HP.tsx:154-155 — There's an empty <p> tag with styles but no content. This renders nothing and should be removed.

---

📝 Accessibility: carousel div used as interactive element

HP.tsx:201-204 — Clickable <div> elements wrapping the carousel cards aren't keyboard-accessible. Use a <button> (or add role="button" + tabIndex={0} + onKeyDown) so keyboard users can cycle through the carousel.

---

📝 No tests

The PR author acknowledged this. The sendHPFeedback utility and the form submission flow need tests given the bug found above.

---

Minor: Props pattern on HPFeedbackForm

HP.tsx:338 passes nameValue="", subjectValue="", feedbackValue="" to the form. Since these are always empty strings, the props add no value — the form initializes its own state from them but could just use useState("") directly. This can be simplified by removing the props entirely.

[hermannsalan]

Converted most of the styles to Tailwind- only the styles relating to transitions in the carousel (lines 32-42) in the homepage remain css~ otherwise gave the homepage its own folder, & removed the html blob from the form :)

[leekahung]

Hey Alan, thanks for the changes! I've requested a few more changes, largely related to styling and implementation, see feedback.

[leekahung]

Similar to letterImages, we can refactor this out of the component

[leekahung]

Refactoring out of the component does not necessary mean out of the component file, both the list and the component can be defined separately in the file with the component referencing the list.

[leekahung]

can you explain why you're removing it and whether there is a visual/no visual change with regards to this change?

[leekahung]

Not really an issue and it's not urgent, but I want to keep a note that we'll probably want to move the other pages like Chat, Letter, About, etc. into the pages directory similar to what is being done with HomePage to keep the root directory of frontend to just configs, styling files, main.tsx, and App.tsx

[leekahung]

@claude can you do another review?

[claude]

Claude finished @leekahung's task in 2m 17s —— View job

---

PR Review — Homepage (#305)

• Gather context
• Read changed files
• Review code quality, bugs, and conventions
• Post final review

---

Good progress on this iteration! The file structure is much better, the form submission bug is fixed, and import paths are corrected. Here's what's still open from @leekahung's second review plus a few new things I caught.

---

Still open from @leekahung's review

⚠️ letterImages and roadmapItems should live outside the component

HomePage.tsx:20,26-29 — These are static arrays that don't depend on props or state. Recreating them on every render is wasteful and makes the component harder to scan. Move them to module scope above const HomePage = () => {.

Fix this →

---

⚠️ HPFeedbackForm props can be removed

HomePage.tsx:284 / HPFeedbackForm.tsx:4-13 — The props nameValue="", subjectValue="", feedbackValue="" are always empty strings, so the form's useState(nameValue) is equivalent to useState(""). Remove the props interface and the three prop args, and initialize state directly with useState("") in the form. Fix this →

---

⚠️ Form doesn't handle errors or give user feedback

HPFeedbackForm.tsx:19-26 — sendHPFeedback is async and returns { success, message }, but the call is not await-ed and the return value is ignored. The user gets no confirmation that their message was sent (or that it failed).

At minimum:

onSubmit={async (event) => {
  event.preventDefault();
  const result = await sendHPFeedback(name, subject, feedback);
  if (!result.success) {
    console.error("Failed to submit feedback:", result.message);
    // TODO: show user-facing error when toasts are available
    return;
  }
  setName("");
  setSubject("");
  setFeedback("");
}}

This also addresses @leekahung's request for a try/catch (the helper already handles that internally and returns the error in result).

Fix this →

---

⚠️ Unused packages: gsap and lucide-react

package.json:23-24 — Both are listed as dependencies but nothing in the codebase imports them. Remove both. Fix this →

---

⚠️ Comparison table should be broken into sub-components

HomePage.tsx:208-244 — The table is 37 lines of nearly identical <div> cells. A small TableRow or CompareRow sub-component in pages/HomePage/ would make this much easier to maintain. Fix this →

---

⚠️ Custom Tailwind values that have standard equivalents

HomePage.tsx — Several custom values can be replaced with standard Tailwind utilities as @leekahung noted. A few specific ones:

Current	Simplified
w-[12px]	w-3
z-[1]	z-1
z-[2]	z-2
z-[3]	z-3
z-[30]	z-30
z-[10]	z-10

Reserve [...] syntax for values that have no Tailwind equivalent (custom hex colors, clamp(...), complex shadows, the responsive selector hack on line 208).

---

❓ DisclaimerLayout.tsx — why was <p> removed?

The diff shows the wrapping <p> tags were removed. This is a shared component used elsewhere in the app. Removing the block-level <p> changes spacing behavior anywhere DisclaimerLayout is rendered. Can you explain the motivation? If the homepage doesn't use DisclaimerLayout at all, the change should be reverted to avoid unintended side effects on other pages.

---

New issues

🐛 Carousel map uses index as key instead of img.id

HomePage.tsx:141 — The map parameter is (img, id) where id is the numeric index, not img.id. This uses an array index as a React key, which is what the previous review flagged:

// current — uses array index
{letterImages.map((img, id) => (
  <div key={id} ...>

// fix — use the stable id field you already defined
{letterImages.map((img) => (
  <div key={img.id} ...>

Fix this →

---

🐛 PageLayout clips the homepage

PageLayout.tsx:10 — isChatPages is true for pathname === "/", which means the homepage gets h-dvh (100% viewport height). This will clip all the content below the fold since the page can't scroll. This needs either a fix in PageLayout to not treat / as a chat-style page, or the homepage needs to opt out. Fix this →

---

📝 Empty <p> tag in hero section

HomePage.tsx:64 — <p className="mt-[25px] ..."></p> renders nothing. It should either be populated with content or removed.

---

📝 clsx used with single static strings

HomePage.tsx:92,94,103,107 — clsx("flex gap-[30px]") with a single string is just noise; clsx adds value only when you have conditional or multiple classes. Use plain className="..." there.

---

📝 roadmapItems.status field is unused

HomePage.tsx:27-29 — Each roadmap item has a status: "complete" | "active" field, but the rendering logic doesn't read it — it uses i === 0 to decide whether to show the connector line. Either use the status field in the rendering, or remove it.

---

📝 mainRef is defined but never used for anything

HomePage.tsx:18 — const mainRef = useRef(null) is attached to the root div but never read or passed anywhere. Remove it. Fix this →

---

📝 Form inputs missing accessible labels

HPFeedbackForm.tsx:28-53 — The inputs rely solely on placeholder for labeling. Placeholders disappear as soon as the user types and are not reliably announced by screen readers. Add <label> elements (visually hidden if needed) paired with each input via htmlFor / id.

---

[hermannsalan]

Hey! Components broken down- folder for Homepage- the comparison grid could be broken down further, but is separated where static and not~ likewise the homepage. Tailwind is more standardized! And, yes, the form is simpler. I didn't extract too many of the functions from the sub-components because they're tightly related & I thought more visible in the file :)

[leekahung]

Keep up the good work, think we're getting close. See feedback

[leekahung]

This is a good case for import (see feedback on static lists)

[leekahung]

Consider keeping this inside the component file, outside of the component

[leekahung]

Made comments on what would be a good example to extract, but we can keep both here since it'll be easier to maintain both the headers and the row data. Can keep as is.

[leekahung]

@claude can you do a double-take on the PR? Check what's been resolved, and what still needs to be touched on that I've missed.

[claude]

Claude finished @leekahung's task in 2m 39s —— View job

---

PR Double-Take Review — Homepage (#305)

• Gather context
• Read changed files
• Cross-reference against open review items
• Post final review

---

Good progress across the iterations. Here's what's been resolved and what's still open, including a few things that slipped through @leekahung's last review.

---

✅ Resolved since the last review

• Comparison table extracted to ComparisonGrid + ComparisonData files
• brainyFeatures and letterImages moved outside their components
• Carousel key uses img.id instead of array index
• Unused gsap / lucide-react removed from package.json
• Duplicate Navbar.tsx gone
• Form properly awaits sendHPFeedback and checks result.success
• DisclaimerLayout.tsx <p> tag restored

---

❌ Still open from @leekahung's review

BrainyFeatures.tsx and ComparisonData.tsx (and LetterImages.tsx) should be .ts, not .tsx

None of these files contain any JSX — they're plain data files. .tsx is only needed when a file uses JSX syntax. Rename all three:

• BrainyFeatures.tsx → BrainyFeatures.ts
• ComparisonData.tsx → ComparisonData.ts
• LetterImages.tsx → LetterImages.ts

HPFeedbackForm props are still being passed as empty strings

HomePage.tsx:266 — <HPFeedbackForm nameValue="" subjectValue="" feedbackValue="" /> — this was flagged in both review rounds and is still unchanged. The Props interface and the three prop args in HPFeedbackForm.tsx:5-9,20-24 should be removed; state should be initialized directly with useState(""). Fix this →

brainyFeatures and letterImages — separate files vs. inline

@leekahung clarified that "refactoring out of the component" means outside the component function but inside the same file. The current approach of separate files (BrainyFeatures.ts, LetterImages.ts) does work, but leekahung's preference is to inline them at the top of HomePage.tsx and LetterCarousel.tsx respectively, since they're simple static arrays. Worth a quick discussion to align on which approach to go with.

---

❌ Things @leekahung hasn't flagged yet that still need fixing

🐛 PageLayout.tsx:10 clips the homepage

isChatPages includes pathname === "/", which applies h-dvh to the homepage. A full-height viewport on a long scrolling page means everything below the fold is clipped and unreachable. This is a real rendering bug.

// current — clips homepage
const isChatPages = pathname === "/" || pathname.startsWith("/letter");

// fix — homepage is a scroll page, not a chat-layout page
const isChatPages = pathname.startsWith("/chat") || pathname.startsWith("/letter");

Fix this →

🐛 mainRef is defined but never used

HomePage.tsx:1,14,19 — useRef is imported, mainRef is created and attached to the root div, but it's never read or passed anywhere. Remove the import, the ref, and the ref={mainRef} attribute. Fix this →

⚠️ Empty <p> tag in hero section

HomePage.tsx:39 — <p className="mt-[25px] ..."></p> has styles but no content. It renders as invisible empty space. Either add content or remove it. Fix this →

⚠️ No console.error on form failure

HPFeedbackForm.tsx:41-43 — when result.success is false, nothing happens. leekahung's review 2 explicitly asked for a console.error in the failure path as a minimum until toasts are available:

if (result.success) {
  setForm({ name: "", subject: "", feedback: "" });
} else {
  console.error("Failed to submit feedback:", result.message);
}

Fix this →

⚠️ clsx used without any conditional logic

HomePage.tsx:74,76,86,90 — several clsx(...) calls wrap a single static string with no conditions. clsx adds no value here; plain className="..." is cleaner. The two-string calls on lines 92 and 98 could also be collapsed into one string.

📝 Form inputs missing accessible labels

HPFeedbackForm.tsx:52-77 — all three inputs rely on placeholder alone. Placeholders disappear on input and aren't reliably read by screen readers. Each input needs a <label> element (can be visually hidden with sr-only) associated via htmlFor/id.

---

📝 No tests

Still none. The form submission flow and sendHPFeedback helper are the most important paths to cover.

---

[leekahung]

Here are some more requests, see feedback.

Also Claude made a good point about PageLayout, those styles would need to be applied conditionally to the chat pages only. Now that Chat it's no longer /, but /chat, the conditional needs to be updated.

[leekahung]

We don't have pre-commit hooks set up for linting and formatting unfortunately, for the time being, run the npm command for lint and format in the CLI, that should patch up the formatting issues along with any linting problems.

[leekahung]

@claude can you perform another review?

[claude]

Claude finished @leekahung's task in 2m 24s —— View job

---

PR Review — Homepage (#305)

• Gather context
• Read changed files
• Cross-reference against open review items
• Post final review

---

✅ Resolved since last review

• HPFeedbackForm props interface removed — form state initialized directly with useState
• console.error on form failure added
• useRef/mainRef removed
• BrainyFeatures.ts, ComparisonData.ts, LetterImages.ts — correctly use .ts extension
• Carousel map correctly uses key={img.id} (not array index)

---

❌ Still open from @leekahung's latest review

clsx misuse throughout — not resolved

HPFeedbackForm.tsx:5-12 — inputClasses is still defined with clsx and no conditionals. leekahung's feedback was to use a plain string constant:

// current — unnecessary clsx
const inputClasses = clsx(
  "m-0 border-none outline-none",
  "bg-[#E6D5B8]/5 text-[#F4F4F2]",
  ...
);

// fix
const inputClasses = "m-0 border-none outline-none bg-[#E6D5B8]/5 text-[#F4F4F2] p-4 text-base rounded-xl transition-colors duration-300 ease-in-out shadow-[inset_0_2px_4px_rgba(255,255,255,0.05),0_1px_3px_rgba(0,0,0,0.1),0_1px_2px_rgba(0,0,0,0.06)] focus:ring-0";

HPFeedbackForm.tsx:65 — clsx(inputClasses, "min-h-40 resize-y") should be a template literal per leekahung's review: `${inputClasses} min-h-40 resize-y`

HPFeedbackForm.tsx:73-77 — the button's clsx(...) with three static strings should be a plain const variable, like inputClasses. Fix this →

HomePage.tsx:71,73,76-80,83,87,89-98 — many clsx("single string") and clsx("string1", "string2") calls with no conditionals. These should all be plain className="...". Fix this →

---

Arrow function components — not resolved

HomePage.tsx:12, ComparisonGrid.tsx:6, LetterCarousel.tsx:5 — all still use arrow function syntax. leekahung flagged this as a consistency issue. The rest of the codebase uses function declarations for components:

// current
const ComparisonGrid = () => { ... };
export default ComparisonGrid;

// fix — matches project convention
export default function ComparisonGrid() { ... }

Same applies to HomePage and LetterCarousel. Fix this →

---

Images in public/ instead of src/assets/ — not resolved

LetterImages.ts:1-3 — imports still point to ../../../../public/:

import LetterTemplate from "../../../../public/LetterTemplate.png";

leekahung's review explicitly called this out. Images should live in src/assets/ so they pass through Vite's bundler (content hashing, tree-shaking, etc). Move the three PNG files to frontend/src/assets/ and update the imports to relative paths. Fix this →

---

❌ Still open from prior reviews (not yet flagged by @leekahung)

🐛 PageLayout.tsx:10 clips the homepage — still not fixed

const isChatPages = pathname === "/" || pathname.startsWith("/letter");

"/" is still in the condition, so the homepage gets h-dvh. This means everything below the fold is unreachable. This is a real rendering bug that's been called out in the last two reviews. Fix this →

---

⚠️ Empty <p> tag in hero section — still not fixed

HomePage.tsx:36 — <p className="mt-[25px] text-[#F4F4F2] text-base leading-[1.8] font-medium"></p> has styles but no content. This has been flagged in two previous reviews. Remove it or add content. Fix this →

---

New issues this round

⚠️ Index used as variable name id in carousel map

LetterCarousel.tsx:49 — letterImages.map((img, id) => ...) — the second argument id is the array index, not the image's id field. The key is correctly set to img.id, but getCardStyle(id) uses the confusingly-named index. This was flagged in the previous review as "fixed" but the variable name is still id. Rename it to index or i to avoid confusion.

⚠️ ComparisonGrid uses index as key

ComparisonGrid.tsx:14,26 — both headers.map((header, idx) and comparisonData.map((row, idx) use idx (array index) as the React key. For the headers array this is acceptable since it's static. For comparisonData, consider adding an id field to the data objects (similar to brainyFeatures and letterImages).

📝 brainyFeatures and letterImages in separate files

@leekahung's latest comment clarified that "refactoring out of the component" means above the component in the same file, not a separate file. BrainyFeatures.ts and LetterImages.ts currently work as separate files, but worth aligning with leekahung on whether to keep them as-is or inline them.

📝 Form inputs still missing accessible labels

HPFeedbackForm.tsx:44-69 — all three inputs still rely solely on placeholder for labeling. This has been open since the first review. Each input needs a <label> (can be sr-only visually) tied via htmlFor/id.

📝 No tests

Still no tests for sendHPFeedback or the form submission flow.

---

[leekahung]

Hey Alan, think for my end, there are just 2 things.

First the contrast between the submit button and background color is too small. Could we apply our existing theme colors for text color and background instead?

Second, the submit functionality for the home page form doesn't seem to work, and it's likely due to it not having the transcript (which make sense in our case for the home page). So for that, could you adjust the backend function for /api/feedback?

So instead of:

      if not file:
         return "No file provided", 404

Try something like:

     if not file:                                                                                                
         name = request.form.get("name")
         subject = request.form.get("subject")                                                                   
         email_params = {                  
             "subject": subject or "Homepage Feedback",
             "from_email": os.getenv("SENDER_EMAIL"),
             "to": [os.getenv("RECIPIENT_EMAIL")],                                                               
             "body": f"From: {name}\n\n{feedback}",
         }                                                                                                       
         try:                              
             EmailMessage(**email_params).send()
             return "Message sent", 200                                                                          
         except Exception as e:
             return f"Send failed: {str(e)}", 500

Think that should fix the API endpoint