Full code review changes #222
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const [invoice, customers] = await Promise.all([ | ||
| fetchInvoiceById(id), | ||
| fetchCustomers(), | ||
| ]); |
There was a problem hiding this comment.
Any reason we shouldn't be fetching in parallel here?
| @@ -105,7 +105,7 @@ export async function updateInvoice(prevState: State, formData: FormData) { | |||
| } | |||
|
|
|||
| export async function deleteInvoice(formData: FormData) { | |||
| throw new Error('Failed to Delete Invoice'); | |||
| // throw new Error('Failed to Delete Invoice'); | |||
There was a problem hiding this comment.
Feels like this should be deleted. If this is the "final form" of the app, then it should be in a fully working state. This throw, if a part of the course, should exist in a lower chapter, but not here, IMO.
| </svg> | ||
| <GlobeAltIcon className="h-12 w-12 rotate-[15deg]" /> | ||
| <p className="text-[44px] ">Acme</p> | ||
| </div> |
There was a problem hiding this comment.
Logo could be dramatically simplified using a Heroicon and rotating it just a touch. :chef-kiss:
| @@ -1,38 +0,0 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Duplicate and unused logo.
| -moz-appearance: textfield; | ||
| appearance: textfield; | ||
| } | ||
|
|
There was a problem hiding this comment.
Moved to global CSS instead of an inline style in React.
| @@ -73,7 +73,6 @@ export default function Form({ customers }: { customers: CustomerField[] }) { | |||
| placeholder="Enter USD amount" | |||
| className="peer block w-full rounded-md border border-gray-200 py-2 pl-10 text-sm outline-2 placeholder:text-gray-500" | |||
| aria-describedby="amount-error" | |||
| style={{ MozAppearance: 'textfield' } as React.CSSProperties} | |||
There was a problem hiding this comment.
Moved to global CSS.
| @@ -81,7 +81,6 @@ export default function EditInvoiceForm({ | |||
| placeholder="Enter USD amount" | |||
| className="peer block w-full rounded-md border border-gray-200 py-2 pl-10 text-sm outline-2 placeholder:text-gray-500" | |||
| aria-describedby="amount-error" | |||
| style={{ MozAppearance: 'textfield' } as React.CSSProperties} | |||
There was a problem hiding this comment.
Moved to global CSS.
Great job on this, everyone! 🎉 I didn't find much, but here's a few things that popped out to me when evaluating the entire app from start to finish.
Also, there's quite a few things that are in
publicandapproot that shouldn't be there if this is the "final form". I get why they are there for now, but they should be deleted when we break things up into chapters.