Conversation
|
@TylerAdamMartinez I think that is looking great. My only notes are: can you make some of the fields display 3 columns wide on large screens so there is less vertical distance (like you have for casing info)? And I have a new idea on ordering based on talking with AMP folks. Here is what I'm thinking for order in this section:
For width on large screens: Formation Code | Aquifer Systems | Aquifer types - in one row - maybe? Obviously those one line have to wrap as you see fit when displayed on smaller screens. But I think it would be nice to have less scrolling when the user is on a desktop. Thoughts? |
de36bd6 to
bc34ad3
Compare
|
@codex review |
6fb619d to
83de086
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <Typography variant="h6">Aquifer Systems:</Typography> | ||
| <Typography variant="body1"> | ||
| {well?.aquifers?.map((a) => a.aquifer_system).join(', ') || 'N/A'} |
There was a problem hiding this comment.
Guard optional aquifer list before join in AdditionalWellInformation
Rendering this accordion calls well?.aquifers?.map(...).join(', ') without guarding the join when well.aquifers is undefined or null; in that case the map short-circuits to undefined and .join throws a TypeError, breaking the entire well detail page for wells without aquifer metadata. Consider defaulting to an empty array before joining (as done elsewhere in the app) so wells lacking aquifer data still render.
Useful? React with 👍 / 👎.
src/components/pdf/well.tsx
Outdated
| <LineItem | ||
| title="Casing Materials" | ||
| value={well?.well_casing_materials.join(', ')} |
There was a problem hiding this comment.
Null-check casing materials before joining in Well PDF
The new PDF section calls well?.well_casing_materials.join(', ') without verifying the array exists. For wells where well_casing_materials is undefined or null (common when data is incomplete), this will throw during PDF generation and prevent the document from rendering. Default to an empty array or skip the line item when the field is absent.
Useful? React with 👍 / 👎.
src/components/pdf/well.tsx
Outdated
| <LineItem | ||
| title="Aquifer Systems" | ||
| value={well?.aquifers?.map((a) => a.aquifer_system).join(', ')} |
There was a problem hiding this comment.
Null-check aquifer systems before joining in Well PDF
Similar to casing materials, the PDF now calls well?.aquifers?.map((a) => a.aquifer_system).join(', ') without guarding the join. If aquifers is missing for a well, the map returns undefined and the subsequent join throws, causing PDF generation to fail for wells lacking aquifer metadata. Use a default empty array or optional chaining on the join to keep PDF creation resilient.
Useful? React with 👍 / 👎.
chasetmartin
left a comment
There was a problem hiding this comment.
I think it's looking really good. It looks like Codex had a few suggestions that do seem relevant before merging.
|
Fixed codex reviews. I still need to create and merge two more PRs to fix the Vite and Cypress tests, respectively. |
…ller & larger devices
9b372b1 to
179484a
Compare
chasetmartin
left a comment
There was a problem hiding this comment.
The added guards look good






Why
This PR addresses the following problem/context:
How
Implementation summary - the following was changed/added/removed:
Notes
Any special considerations, workarounds, or follow-up work to note?
staging