Conversation
Tarun-Nagesh
changed the title
Juwang110
left a comment
Juwang110
left a comment
There was a problem hiding this comment.
See requested changes and comments, everything seems to be here but there are some frontend changes. See slack messages as well for more frontend stuff, Thanks!
| @@ -1,4 +1,5 @@ | |||
| import React, { useEffect, useState } from 'react'; | |||
| import { useNavigate } from 'react-router-dom'; | |||
There was a problem hiding this comment.
I personally think it's fine to just link the application details with pantry name for now since this page will have to be refactored a lot anyway, so I think it's okay to leave it as you have it unless the other reviewer has a preference
| import ApiClient from '@api/apiClient'; | ||
| import { Pantry } from 'types/types'; | ||
|
|
||
| type PantryWithShipment = Pantry & { |
There was a problem hiding this comment.
I'm a bit confused on the logic with this type and think it can be removed, the pantry type already has the relevant fields shipmentaddress1, etc.
| ); | ||
| } | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
I would ask Priya/Sam or Yurika how we want to display errors like this or if it's fine to leave it for now
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState<string | null>(null); | ||
|
|
||
| const formatPhone = (phone?: string | null) => { |
There was a problem hiding this comment.
we can move this function to src/utils/utils.ts so other places can use it if need be
| <Box bg="neutral.50" minH="100vh" p={8}> | ||
| <Box maxW="1200px" mx="auto"> | ||
| {/* Page Title */} | ||
| <Heading as="h1" textStyle="h1" color="neutral.600" mb={8}> |
There was a problem hiding this comment.
color should be gray: light (#515151)
| <Heading as="h3" textStyle="h4" mb={3}> | ||
| Shipping Address | ||
| </Heading> | ||
| <VStack align="stretch" gap={1}> |
There was a problem hiding this comment.
can we make the vertical gap between this contact info less to match the figma
| <Flex wrap="wrap" gap={2} mb={6}> | ||
| {application.restrictions && application.restrictions.length > 0 ? ( | ||
| application.restrictions.map((restriction, index) => ( | ||
| <Badge |
There was a problem hiding this comment.
add a visible border on these badges to match figma design
| <Flex wrap="wrap" gap={2}> | ||
| {application.activities && application.activities.length > 0 ? ( | ||
| application.activities.map((activity, index) => ( | ||
| <Badge |
| <Grid templateColumns="repeat(2, 1fr)" gap={6}> | ||
| <GridItem> | ||
| <Text textStyle="p2" fontWeight="600" mb={1}> | ||
| Confident in Identifying the Top 9 Allergens |
There was a problem hiding this comment.
I'd ask priya for clarification on figma for this section, in the figma there is a section for Confidence in Identifying the Top 9 Allergens which you don't have and need to add, but there's also two sections for Serves Allergen-Avoidant Children
| > | ||
| Deny | ||
| </Button> | ||
| <Button |
There was a problem hiding this comment.
Color in figma for this button is blue #213C4A
3 participants
ℹ️ Issue
Closes SSF-115
📝 Description
Implemented admin-facing application details page that displays pantry application information and allows admins to approve or deny applications.
Files changed/added:
✔️ Verification
Tested on http://localhost:4200/approve-pantries by clicking through to application details. Verified all application data displays correctly and approve/deny buttons navigate properly.
🏕️ (Optional) Future Work / Notes
For now, (according to the ticket), I just linked the page directly to the application name, but I noticed in the figma that theres a separate column in the "http://localhost:4200/approve-pantries" link, so lmk if I should that instead.