Added features to success page #7
Conversation
components/CheckoutForm.tsx
Outdated
| @@ -23,10 +30,12 @@ export const CheckoutForm = (): React.ReactElement => { | |||
| return; | |||
| } | |||
|
|
|||
| const successURL = "http://localhost:8080/checkout?id=".concat(orderId); | |||
There was a problem hiding this comment.
Use an ENV variable for this since production != development URL
|
|
||
| export const Success: React.VFC = () => { | ||
| const [orderId, setOrderId] = useState<string>(""); | ||
| const router = useRouter(); |
There was a problem hiding this comment.
Is there no typing needed here like <url: string>
Might reduce the need for AS below
pages/success.tsx
Outdated
| const router = useRouter(); | ||
|
|
||
| useEffect(() => { | ||
| const orderIdString: string = router.query.id as string; |
There was a problem hiding this comment.
As string is usually something you want to avoid if you can
There was a problem hiding this comment.
This was used to get around this typescript error, "Type 'string[]' is not assignable to type 'string'." Would this be a case where it is something we can't avoid?
There was a problem hiding this comment.
Why can't you do [0] to grab the first string in the index since that's what it is? Just becuase the override the type doesn't override the underlying JavaScript structure. This will lead to confusion down the road ;).
There was a problem hiding this comment.
When I tried getting the element at index 0 I would only get the first char in the order id which made me think that the id should be a string but for some reason its being identified as an array of string by typescript.
There was a problem hiding this comment.
Ok let's take a look during our meeting. In the meantime I would suggest looking at the typing for Router for the entire application as the answer will probs be there.
Also try removing as string and seeing if the error goes away
pages/success.tsx
Outdated
| }, [router.query]); | ||
|
|
||
| useEffect(() => { | ||
| const timer = setTimeout(() => window.location.replace("/order"), 7000); |
pages/success.tsx
Outdated
| @@ -16,6 +34,11 @@ export const Success: React.VFC = () => { | |||
| textAlign: "center", | |||
| }; | |||
|
|
|||
| const buttonProps = { | |||
| color: "red", | |||
pages/success.tsx
Outdated
| @@ -16,6 +34,11 @@ export const Success: React.VFC = () => { | |||
| textAlign: "center", | |||
| }; | |||
|
|
|||
| const buttonProps = { | |||
| color: "red", | |||
| onClick: () => window.location.replace("/order"), | |||
pages/success.tsx
Outdated
|
|
||
| const StyledButton = styled(Button)` | ||
| margin-left: auto; | ||
| margin-right: auto; |
There was a problem hiding this comment.
Use margin short form notation
components/CheckoutForm.tsx
Outdated
| declare var process: { | ||
| env: { | ||
| BASE_URL: string; | ||
| }; | ||
| }; | ||
|
|
There was a problem hiding this comment.
It will be very tedious for you to declare this everywhere you use process. Have an index.d.TS file
D stands for definitions
components/CheckoutForm.tsx
Outdated
| @@ -23,10 +36,12 @@ export const CheckoutForm = (): React.ReactElement => { | |||
| return; | |||
| } | |||
|
|
|||
| const successURL = process.env.BASE_URL + "/checkout?id=".concat(orderId); | |||
There was a problem hiding this comment.
Use a template literal instead.
pages/success.tsx
Outdated
| } | ||
|
|
||
| useEffect(() => { | ||
| const duration = 7000; |
There was a problem hiding this comment.
Const at top of the file, no need to redeclare.
Duration is a bad variable name.
What is the meaning! The meaning is
SESSION_EXPIRY_TIMEOUT = 7000;
Much more intitutife when you read the variable versus just 7000
pages/success.tsx
Outdated
| const buttonProps = { | ||
| onClick: newOrder, | ||
| }; |
There was a problem hiding this comment.
Remove this and just pass it inline
The order id is now included on the success page along with the button to redirect to the order page. The page is also set to automatically redirect to the order page after 7 seconds regardless using SetTimeout.
Screen.Recording.2022-01-07.at.2.16.19.PM.mov