#3370 - all parts under review container #3424
Conversation
chpy04
left a comment
chpy04
left a comment
There was a problem hiding this comment.
Can you make this look more like the epic? There shouldn't be white boarders and there should be different shades of gray to make it look a bit softer. Take a look at the epic for reference
| // check if current user is head or lead for this project | ||
| const currentUser = useCurrentUser(); | ||
| const { wbsNum } = project; | ||
| const { data } = usePartsFromProject(wbsPipe(wbsNum)); |
There was a problem hiding this comment.
need to check for loading or error in hook
chpy04
left a comment
chpy04
left a comment
There was a problem hiding this comment.
Few small changes. In general, don't return null from a component. Conditional components should be checked in the parent components
| const { data, isLoading, isError, error } = usePartsFromProject(wbsPipe(wbsNum)); | ||
|
|
||
| if (isLoading || isError || error) { | ||
| return null; |
There was a problem hiding this comment.
if its loading, return a loading indicator. If isError is true, return an Error component with the error in it. Take a look at how this is done in other places on the site, this is a super common pattern
| // filter out parts in project that have not been approved | ||
| const parts = data?.filter((part) => part.status !== Review_Status.APPROVED) || []; | ||
| // show list only if head/lead of current project | ||
| if (!(isUserHead || isUserLead) || parts.length === 0) { |
There was a problem hiding this comment.
Don't check for head/lead here. We will check that in the parent component to determine if we want to display this one. If as a result you no longer need the currentUser get rid of that too
| // filter out parts in project that have not been approved | ||
| const parts = data?.filter((part) => part.status !== Review_Status.APPROVED) || []; | ||
| // show list only if head/lead of current project | ||
| if (!(isUserHead || isUserLead) || parts.length === 0) { |
There was a problem hiding this comment.
If the parts.length is 0, we want to convey that to the user. You can do that by returning something like this:
<EmptyPageBlockDisplay
icon={<CheckCircleOutlineOutlinedIcon sx={{ fontSize: 70 }} />}
heading={`You're all caught up!`}
message={'There are no in progress parts on this project!'}
/>
| @@ -1,4 +1,4 @@ | |||
| FROM node:20-alpine | |||
There was a problem hiding this comment.
note to revieweres: this is good with me
added loading, error, removed checking user
GauriRajesh733
changed the title
Changes
added container to display parts in a project that are still being reviewed (every status except approved). this container only appears for heads/leads of a project if there are any parts in the process of being reviewed. the container has dynamic width and height with a scroll bar if needed.
Notes
tested components manually on PartPage.tsx with examples (list of parts that are approved so nothing should display, list of parts that include parts that have not been approved w/ and w/out scroll bar, loading/error), etc..
used component setup from #3369 since the only difference is that this component only appears for leads/heads of projects.
Test Cases
N/A
Screenshots
To Do
i'm not exactly sure how to test this component aside from creating manual examples (ex. i changed currentUser at line 13 in AllPartsUnderReviewContainer.tsx when testing what happens if I change the user to a project lead, head, or member). there are a few example projects in seed.ts but i am not sure how to test using those.
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes # (issue #)