Skip to content

Feature/kanban 1084#1752

Open
Joe-Wdz wants to merge 6 commits intodevelop/ui-wgfrom
feature/KANBAN-1084
Open

Feature/kanban 1084#1752
Joe-Wdz wants to merge 6 commits intodevelop/ui-wgfrom
feature/KANBAN-1084

Conversation

@Joe-Wdz
Copy link
Copy Markdown
Contributor

@Joe-Wdz Joe-Wdz commented Apr 14, 2026

KANBAN-1084 Disease ribbon cell color for single annotations

@Joe-Wdz
Copy link
Copy Markdown
Contributor Author

Joe-Wdz commented Apr 14, 2026

@claude please review my changes

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 14, 2026

Claude finished @Joe-Wdz's task in 1m 31s —— View job


Code Review: PR #1752 — Contact Us Form with reCAPTCHA v3

  • Gather context and read changed files
  • Review code for bugs, security issues, and best practices
  • Post detailed review feedback

Overall this is a solid implementation of a Contact Us form with reCAPTCHA v3 integration. The form UX is clean, validation is sensible, and the topBar refactor from class to functional component is a nice improvement. Here are the issues I found:


Bug: Typo in default error message

src/lib/sendContactEmail.js:10

let errMessage = 'Failed to send errMessage.';

This should be 'Failed to send message.' — "errMessage" is a variable name that leaked into the user-facing string.

Fix this →


Bug: useEnterprise prop may be incorrect

src/containers/contactPage/index.jsx:177

<GoogleReCaptchaProvider reCaptchaKey={RECAPTCHA_SITE_KEY} useEnterprise>

The useEnterprise prop tells the library to load the reCAPTCHA Enterprise script. If the site key (6Ld-r5EsAAAA...) is a standard reCAPTCHA v3 key (not Enterprise), this will cause the captcha to fail silently or throw errors. Please verify that the reCAPTCHA key is actually an Enterprise key. If it's a standard v3 key, remove the useEnterprise prop:

<GoogleReCaptchaProvider reCaptchaKey={RECAPTCHA_SITE_KEY}>

Cleanup: Remove commented-out code

src/containers/contactPage/index.jsx:19

// const EMAIL_PATTERN = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;

This commented-out regex is unused dead code (the form correctly uses the browser's native type="email" validation instead). It should be removed before merging.

Fix this →


Suggestion: Clear form fields only on success, not on reset

src/containers/contactPage/index.jsx:55-62

On successful submit, the form clears all fields (lines 59-62). This duplicates the logic in handleReset (lines 36-43). Consider calling handleReset() inside the success block to avoid duplication. Minor nit — not a blocker.


Note: PR title/branch mismatch

The branch is feature/KANBAN-1084 and the PR description references "Disease ribbon cell color for single annotations", but the actual changes implement a Contact Us form (KANBAN-462 per commit messages). This may cause confusion in issue tracking. Consider updating the PR title to reflect the actual changes.


Summary

Category Items
Bugs 2 (typo in error message, possibly wrong useEnterprise flag)
Cleanup 1 (commented-out code)
Suggestions 1 (DRY up form reset)
Notes 1 (PR title mismatch)

The topBar refactor to a functional component and the routing/navigation changes look correct. The form validation, error handling, and reCAPTCHA flow are well structured. Please address the bug items above before merging.


|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant