-
Notifications
You must be signed in to change notification settings - Fork 1
Added features to success page #7
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,28 @@ | ||
import { Card, Heading, SmallText } from "@cheapreats/react-ui"; | ||
import { Card, Heading, SmallText, Button } from "@cheapreats/react-ui"; | ||
import React from "react"; | ||
import styled from "styled-components"; | ||
import Image from "next/image"; | ||
import Snowfall from "react-snowfall"; | ||
import { useState, useEffect } from "react"; | ||
import { useRouter } from "next/router"; | ||
|
||
export const Success: React.VFC = () => { | ||
const [orderId, setOrderId] = useState<string>(""); | ||
const router = useRouter(); | ||
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. Is there no typing needed here like <url: string> |
||
|
||
useEffect(() => { | ||
const orderIdString: string = router.query.id as string; | ||
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. As string is usually something you want to avoid if you can 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. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
if (!!router.query) { | ||
setOrderId(orderIdString); | ||
} | ||
}, [router.query]); | ||
|
||
useEffect(() => { | ||
const timer = setTimeout(() => window.location.replace("/order"), 7000); | ||
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. 7000 is a magic number |
||
return () => clearTimeout(timer); | ||
}, []); | ||
|
||
const headingProps = { | ||
color: "green", | ||
textAlign: "center", | ||
|
@@ -16,6 +34,11 @@ export const Success: React.VFC = () => { | |
textAlign: "center", | ||
}; | ||
|
||
const buttonProps = { | ||
color: "red", | ||
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. Put this in styled component |
||
onClick: () => window.location.replace("/order"), | ||
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. Just create a function |
||
}; | ||
|
||
return ( | ||
<div | ||
style={{ | ||
|
@@ -30,9 +53,13 @@ export const Success: React.VFC = () => { | |
<Image src={require("../images/logo.jpg")} /> | ||
<Heading {...headingProps}>Thanks for your order!</Heading> | ||
<SmallText {...smallTextProps}> | ||
We appreciate your business! If you have any questions, please email | ||
Your order id is {orderId}. We appreciate your business! If you have | ||
any questions, please email | ||
<a href="mailto:[email protected]"> [email protected]</a> | ||
</SmallText> | ||
<StyledButton primary {...buttonProps}> | ||
Start New Order | ||
</StyledButton> | ||
</StyledCard> | ||
</div> | ||
); | ||
|
@@ -49,3 +76,10 @@ const StyledCard = styled(Card)` | |
postion: relative; | ||
padding: 3em; | ||
`; | ||
|
||
const StyledButton = styled(Button)` | ||
margin-left: auto; | ||
margin-right: auto; | ||
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. Use margin short form notation |
||
text-align: center; | ||
margin-top: 1em; | ||
`; |
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.
Use an ENV variable for this since production != development URL