-
Notifications
You must be signed in to change notification settings - Fork 0
[SSF-115] Admin application details #91
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
base: main
Are you sure you want to change the base?
Conversation
Juwang110
left a comment
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
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.
same comment as before with border
| <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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Color in figma for this button is blue #213C4A
ℹ️ 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.