Project 3 - Frontend (Ian Lau, Qian Ling, Charles Lee)#48
Project 3 - Frontend (Ian Lau, Qian Ling, Charles Lee)#48charlesleezhaoyi wants to merge 172 commits into
Conversation
…e/re-route accordingly
Test forum navbar + Initial FE auth logic
Add function in tailwind.config.js for component checking
…king profile picture
…k of navbar profile picture
Test dashboard
…dded error alert on onboarding when user has not verified email
Onboarding + Dashboard
added API calls to fetch token + send verification email on clicking …
Test - navbar.js + app.js
Test final fix
| }) { | ||
| const { getAccessTokenSilently } = useAuth0(); | ||
| const { bookId } = useParams(); | ||
| const handleSubmit = async () => { |
There was a problem hiding this comment.
naming a bit odd. This should be handleClick rather, since we do not submit a form in this component
| }, | ||
| { headers: { Authorization: `Bearer ${token}` } } | ||
| ); | ||
| window.location.reload(); |
There was a problem hiding this comment.
By right we should avoid reloads like such, as it goes against react principles
| @@ -0,0 +1,15 @@ | |||
| const Button = ({ label, onClick }) => { | |||
There was a problem hiding this comment.
You could have used this for the AcceptRequestButton instead of writing another component. This could be made generic enough to fit the AcceptRequestButton criteria
| setSelectedCategories((prevSelected) => { | ||
| if (prevSelected.includes(category)) { | ||
| // If it is, remove it from the array | ||
| return prevSelected.filter((c) => c !== category); |
| key={category} | ||
| onClick={(e) => toggleCategory(e, category)} | ||
| className={`px-4 py-2 rounded-full border ${ | ||
| selectedCategories.includes(category) |
There was a problem hiding this comment.
Could we solve this in another way? Looping through a whole array just to apply styles seems a bit overkill
| import useLoadCategories from "../hooks.js/useLoadCategories.js"; | ||
| import ArrowBackRoundedIcon from "@mui/icons-material/ArrowBackRounded"; | ||
| import Photo from "../components/Common/Photo.js"; | ||
| const defaultInput = { |
There was a problem hiding this comment.
If we don't reuse this, why not just place the object in the default state directly?
| <TextInput | ||
| label="Book title" | ||
| type="text" | ||
| onChange={(e) => | ||
| setInputData((prev) => { | ||
| return { ...prev, title: e.target.value }; | ||
| }) | ||
| } | ||
| value={inputData.title} | ||
| placeholder="Grumpy Monkey" | ||
| /> | ||
| <TextInput | ||
| label="Author" | ||
| type="text" | ||
| onChange={(e) => | ||
| setInputData((prev) => { | ||
| return { ...prev, author: e.target.value }; | ||
| }) | ||
| } | ||
| value={inputData.author} | ||
| placeholder="Suzanne Lang" | ||
| /> |
There was a problem hiding this comment.
I see a lot of repetition here that we could easily handle dynamically
| setPhotoURLs( | ||
| photos.map((photo) => convertBufferToPhoto(photo.file.data)) | ||
| ); | ||
| const requestRes = await axios.get( |
There was a problem hiding this comment.
I would almost want to create a second useEffect here to separate responsibilities
|
|
||
| const photosDisplay = photoURLs.map((url) => <Photo url={url} key={url} />); | ||
|
|
||
| const nonDonorRequestDisplay = isBeneRequested ? ( |
There was a problem hiding this comment.
Let's write out words like Bene. This is not intuitive to read for everyone and takes up brain capacity to spell out
| return isLoadingData ? ( | ||
| <Loading /> | ||
| ) : ( |
There was a problem hiding this comment.
I think two return statements might be easier to read here :)
|
|
||
| https://www.figma.com/file/BltTY2KqZ9q43BWp3k065P/Book-exchange-app?type=design&node-id=0-1&mode=design&t=IHFmjkw6Xp3dVCND-0 | ||
|
|
||
| ## How to install the app |
There was a problem hiding this comment.
What about the environment variables needed to setup?
| </div> | ||
| <div className="flex flex-col w-5/6 space-y-3"> | ||
| <label className="text-xl self-start">Genres:</label> | ||
| {!!categoryOptions.length ? ( |
There was a problem hiding this comment.
| {!!categoryOptions.length ? ( | |
| {categoryOptions.length ? ( |
4 participants
No description provided.