-
-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
created link button to new item #227
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,14 @@ | ||
'use client'; | ||
import Link from 'next/link'; | ||
import ButtonRounded from '@/components/buttons/ButtonRounded'; | ||
import ButtonGoToItem from '@/components/buttons/ButtonGoToItem'; | ||
|
||
const SuccessPage = () => { | ||
import React from 'react'; | ||
import { useSearchParams } from 'next/navigation'; | ||
|
||
const SuccessPage = async () => { | ||
const searchParams = useSearchParams(); | ||
const idItem = searchParams.get('id'); | ||
return ( | ||
<div className='my-10 flex flex-col items-center gap-5'> | ||
<h1 className='text-center text-xl font-bold'> | ||
|
@@ -10,6 +17,9 @@ const SuccessPage = () => { | |
<Link href='/add-item'> | ||
<ButtonRounded type='button'>ADD ANOTHER ITEM</ButtonRounded> | ||
</Link> | ||
<Link href={`/item/${idItem}`}> | ||
<ButtonGoToItem type='button'>GO TO YOUR ITEM</ButtonGoToItem> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need a new component here. The |
||
</Link> | ||
</div> | ||
); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
'use client'; | ||
import React from 'react'; | ||
|
||
type ButtonRoundedPropTypes = { | ||
children: string; | ||
type: 'button' | 'submit' | 'reset'; | ||
isDisabled?: boolean; | ||
clickHandler?: (event: React.MouseEvent<HTMLButtonElement>) => void; | ||
}; | ||
|
||
const ButtonGoToItem: React.FC<ButtonRoundedPropTypes> = ({ | ||
children, | ||
clickHandler, | ||
isDisabled, | ||
type, | ||
}) => { | ||
return ( | ||
<button | ||
className='button button-rounded button-stroke disabled:bg-hoverGreen' | ||
onClick={clickHandler} | ||
role='button' | ||
type={type} | ||
disabled={isDisabled} | ||
aria-label={`${children}`} | ||
> | ||
{children} | ||
</button> | ||
); | ||
}; | ||
|
||
export default ButtonGoToItem; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,7 +7,9 @@ export default async function insertRow( | |||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||
// setSubmitting(true); | ||||||||||||||||||||||||||
const supabase = newClient(); | ||||||||||||||||||||||||||
await supabase.from(table).insert(InsertValues).select(); | ||||||||||||||||||||||||||
const response = await supabase.from(table).insert(InsertValues).select(); | ||||||||||||||||||||||||||
const [createdRecord] = response?.data ?? []; | ||||||||||||||||||||||||||
return createdRecord as PartialItem; | ||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||
// setSubmitting(false); | ||||||||||||||||||||||||||
console.log('An unexpected error occurred'); | ||||||||||||||||||||||||||
Comment on lines
+10
to
15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Using the nullish coalescing operator here will mean we always get an array even if there was an error - we definitely don't want that because it would hide any issues happening in our query to the database. Instead, we can refactor the function to actually throw the error back up to be handled appropriately wherever we called this query. |
||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the refactor for
insertRow
suggested in this review let's also adapt this implementation of the function so we can handle the error it would throw in case of a failure. It should just be a case of replicating the same pattern.💡 Also, I really liked the fact that you attempted to enforce our types in your refactor of
insertRow
but let's move that declaration up to here instead.I'm suggesting this because I think it will be a better separation of concerns - it means we let our util return the data just as is and then we get TypeScript to evaluate if it matches the type we expected once it's been returned here. That way the static testing will be working for us and alerting us if anything changes in future and our Types need a review or there is an issue with the data returned.