Skip to content

Commit a6dddd6

Browse files
bderbs30bderbs30
committed
#1637: code quality and wbs links
1 parent a279e00 commit a6dddd6

2 files changed

Lines changed: 104 additions & 90 deletions

File tree

‎src/frontend/src/components/ChangeRequestDetailCard.tsx‎

Lines changed: 84 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* See the LICENSE file in the repository root folder for details.
44
*/
55

6-
import { Card, CardContent, Grid, Typography, useTheme } from '@mui/material';
6+
import { Card, CardContent, Grid, Hidden, Typography, useTheme } from '@mui/material';
77
import Chip from '@mui/material/Chip';
88
import { green, blue, red, grey, purple } from '@mui/material/colors';
99
import { Box, Stack } from '@mui/system';
@@ -14,66 +14,90 @@ import { Link as RouterLink } from 'react-router-dom';
1414
import { fullNamePipe } from '../utils/pipes';
1515
import { ChangeRequestTypeTextPipe, ChangeRequestStatusTextPipe } from '../utils/enum-pipes';
1616

17+
// Might not be the best way to do this. Open for suggestions.
1718
const determineChangeRequestTypeView = (cr: ChangeRequest) => {
18-
switch (cr.status) {
19-
case 'Implemented':
20-
return <ImplementedCardDetails cr={cr} />;
19+
switch (cr.type) {
20+
case ChangeRequestType.Activation: // could just be done in an if with && let me know your thoughts
21+
case ChangeRequestType.StageGate:
22+
return cr.status === ChangeRequestStatus.Implemented ? (
23+
<ImplementedCardDetails cr={cr} />
24+
) : (
25+
<StageGateActivationCardDetails cr={cr} />
26+
);
2127
default:
2228
return <StandardCardDetails cr={cr as StandardChangeRequest} />;
2329
}
2430
};
2531

32+
// should I use object mapping instead of switch statements?
2633
const determineChangeRequestStatusPillColor = (status: ChangeRequestStatus) => {
2734
switch (status) {
28-
case 'Implemented':
35+
case ChangeRequestStatus.Implemented:
2936
return blue[600];
30-
case 'Accepted':
37+
case ChangeRequestStatus.Accepted:
3138
return green[600];
32-
case 'Denied':
39+
case ChangeRequestStatus.Denied:
3340
return red[400];
34-
case 'Open':
41+
case ChangeRequestStatus.Open:
3542
return purple[400];
3643
default:
3744
return grey[500];
3845
}
3946
};
4047

48+
// same for here ^^
4149
const determineChangeRequestTypeDesc = (type: ChangeRequestType) => {
4250
switch (type) {
43-
case 'ISSUE':
44-
return 'Issue';
45-
case 'DEFINITION_CHANGE':
46-
return 'Redefinition';
47-
case 'OTHER':
48-
return 'Other';
49-
case 'STAGE_GATE':
51+
case ChangeRequestType.StageGate:
5052
return 'Stage Gate';
51-
case 'ACTIVATION':
52-
return 'Activation';
53+
case ChangeRequestType.Activation:
54+
return 'Activate';
5355
default:
5456
return 'Other';
5557
}
5658
};
5759

60+
// return the review notes for a change request if there are any otherwise display No notes
5861
const ImplementedCardDetails = ({ cr }: { cr: ChangeRequest }) => {
5962
const theme = useTheme();
60-
const DescriptionType = determineChangeRequestTypeDesc(cr.type);
6163
return (
6264
<Box
6365
sx={{
6466
backgroundColor: theme.palette.divider,
6567
width: '100%',
66-
minHeight: 75,
68+
height: 75,
69+
borderRadius: 1,
70+
padding: 1,
71+
overflow: 'hidden',
72+
textOverflow: 'ellipsis'
73+
}}
74+
>
75+
<Typography variant="body1" fontSize={14}>
76+
{cr.reviewNotes ? cr.reviewNotes : 'No review notes'}
77+
</Typography>
78+
</Box>
79+
);
80+
};
81+
82+
// could possibly abstract these as the contents are the only difference
83+
// non implemented change requests that are of stage gat and activation type
84+
const StageGateActivationCardDetails = ({ cr }: { cr: ChangeRequest }) => {
85+
const theme = useTheme();
86+
const descriptionType = determineChangeRequestTypeDesc(cr.type);
87+
return (
88+
<Box
89+
sx={{
90+
backgroundColor: theme.palette.divider,
91+
width: '100%',
92+
height: 75,
6793
borderRadius: 1,
68-
marginTop: 1,
69-
paddingTop: 0.5,
70-
paddingLeft: 1,
71-
paddingRight: 1,
72-
paddingBottom: 0.5
94+
padding: 1,
95+
overflow: 'hidden',
96+
textOverflow: 'ellipsis'
7397
}}
7498
>
75-
<Typography variant="body1" fontSize={14} noWrap>
76-
{DescriptionType + ': '}
99+
<Typography variant="body1" fontSize={14}>
100+
{descriptionType + ': '}
77101
<Link color="inherit" component={RouterLink} to={`${routes.PROJECTS}/${wbsPipe(cr.wbsNum)}`}>
78102
{wbsPipe(cr.wbsNum)}
79103
</Link>
@@ -89,20 +113,21 @@ const StandardCardDetails = ({ cr }: { cr: StandardChangeRequest }) => {
89113
sx={{
90114
backgroundColor: theme.palette.divider,
91115
width: '100%',
92-
minHeight: 75,
116+
height: 75,
93117
borderRadius: 1,
94-
marginTop: 1,
95-
paddingTop: 0.5,
96-
paddingLeft: 1,
97-
paddingRight: 1,
98-
paddingBottom: 0.5
118+
padding: 1,
119+
overflow: 'hidden',
120+
textOverflow: 'ellipsis'
99121
}}
100122
>
101-
{cr.what}
123+
<Typography variant="body1" fontSize={14}>
124+
{cr.what}
125+
</Typography>
102126
</Box>
103127
);
104128
};
105129

130+
// I can abstract the two pills into one component but I'm not sure if it's worth it since the difference is the label and color
106131
const ChangeRequestTypePill = ({ type }: { type: ChangeRequestType }) => {
107132
return (
108133
<Chip
@@ -113,7 +138,6 @@ const ChangeRequestTypePill = ({ type }: { type: ChangeRequestType }) => {
113138
fontSize: 12,
114139
color: 'white',
115140
backgroundColor: red[600],
116-
mb: 0.5,
117141
width: 100
118142
}}
119143
/>
@@ -131,48 +155,12 @@ const ChangeRequestStatusPill = ({ status }: { status: ChangeRequestStatus }) =>
131155
fontSize: 12,
132156
color: 'white',
133157
backgroundColor: statusPillColor,
134-
mb: 0.5,
135158
width: 100
136159
}}
137160
/>
138161
);
139162
};
140163

141-
const ChangeRequestCardHeader = ({ cr }: { cr: ChangeRequest }) => {
142-
return (
143-
<Link underline={'none'} color={'inherit'} component={RouterLink} to={`${routes.CHANGE_REQUESTS}/${cr.crId}`} noWrap>
144-
<Typography variant="h6" sx={{ mb: 0.5 }}>
145-
{'Change Request #' + cr.crId}
146-
</Typography>
147-
</Link>
148-
);
149-
};
150-
151-
const ChangeRequestStatusTypePills = ({ cr }: { cr: ChangeRequest }) => {
152-
return (
153-
<Stack direction={'column'} spacing={1}>
154-
<ChangeRequestTypePill type={cr.type} />
155-
<ChangeRequestStatusPill status={cr.status} />
156-
</Stack>
157-
);
158-
};
159-
160-
const ChangeRequestSubWBSDetails = ({ cr }: { cr: ChangeRequest }) => {
161-
return (
162-
<Stack direction={'column'}>
163-
<Typography variant="body1" sx={{ mr: 2, fontWeight: 'bold', fontSize: 13 }}>
164-
From: {fullNamePipe(cr.submitter)}
165-
</Typography>
166-
<Typography fontWeight={'bold'} variant="h1" fontSize={13} noWrap>
167-
WBS:{' '}
168-
<Link color={'inherit'} component={RouterLink} to={`${routes.PROJECTS}/${wbsPipe(cr.wbsNum)}`}>
169-
{wbsPipe(cr.wbsNum)} {cr.wbsName}
170-
</Link>
171-
</Typography>
172-
</Stack>
173-
);
174-
};
175-
176164
interface ChangeRequestDetailCardProps {
177165
changeRequest: ChangeRequest;
178166
}
@@ -185,11 +173,33 @@ const ChangeRequestDetailCard: React.FC<ChangeRequestDetailCardProps> = ({ chang
185173
<CardContent>
186174
<Grid container justifyContent="space-between" alignItems="flex-start">
187175
<Grid item xs mb={1} mt={-1.5}>
188-
<ChangeRequestCardHeader cr={changeRequest} />
189-
<ChangeRequestSubWBSDetails cr={changeRequest} />
176+
<Link
177+
underline={'none'}
178+
color={'inherit'}
179+
component={RouterLink}
180+
to={`${routes.CHANGE_REQUESTS}/${changeRequest.crId}`}
181+
>
182+
<Typography variant="h6" sx={{ mb: 0.5 }}>
183+
{'Change Request #' + changeRequest.crId}
184+
</Typography>
185+
</Link>
186+
<Stack direction={'column'}>
187+
<Typography variant="body1" sx={{ mr: 2, fontWeight: 'bold', fontSize: 13 }}>
188+
From: {fullNamePipe(changeRequest.submitter)}
189+
</Typography>
190+
<Typography fontWeight={'bold'} variant="h1" fontSize={13} noWrap>
191+
WBS:{' '}
192+
<Link color={'inherit'} component={RouterLink} to={`${routes.PROJECTS}/${wbsPipe(changeRequest.wbsNum)}`}>
193+
{wbsPipe(changeRequest.wbsNum)} {changeRequest.wbsName}
194+
</Link>
195+
</Typography>
196+
</Stack>
190197
</Grid>
191-
<Grid item xs="auto">
192-
<ChangeRequestStatusTypePills cr={changeRequest} />
198+
<Grid item xs="auto" mb={1}>
199+
<Stack direction={'column'} spacing={1}>
200+
<ChangeRequestTypePill type={changeRequest.type} />
201+
<ChangeRequestStatusPill status={changeRequest.status} />
202+
</Stack>
193203
</Grid>
194204
</Grid>
195205
<ChangeRequestTypeView />

‎src/frontend/src/pages/ChangeRequestsPage/ChangeRequestsOverview.tsx‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,28 @@ const ChangeRequestsOverview: React.FC = () => {
107107
</Box>
108108
);
109109

110+
const renderChangeRequests = (title: string, crList: ChangeRequest[], emptyMessage: string) => {
111+
return (
112+
<>
113+
<Typography variant="h5" gutterBottom>
114+
{title}
115+
</Typography>
116+
{crList.length > 0 ? (
117+
<Grid container>{displayCRCards(crList)}</Grid>
118+
) : (
119+
<Typography variant="subtitle2" gutterBottom>
120+
{emptyMessage}
121+
</Typography>
122+
)}
123+
</>
124+
);
125+
};
126+
110127
return (
111128
<Box>
112-
{showToReview && (
113-
<>
114-
<Typography variant="h5" gutterBottom>
115-
To Review
116-
</Typography>
117-
<Grid container>{displayCRCards(crToReview)}</Grid>
118-
</>
119-
)}
120-
<Typography variant="h5" gutterBottom>
121-
My Un-reviewed Change Requests
122-
</Typography>
123-
<Grid container>{displayCRCards(crUnreviewed)}</Grid>
124-
<Typography variant="h5" gutterBottom>
125-
My Recently Approved Change Requests
126-
</Typography>
127-
<Grid container>{displayCRCards(crApproved)}</Grid>
129+
{showToReview && renderChangeRequests('To Review', crToReview, 'No change requests to review')}
130+
{renderChangeRequests('My Un-reviewed Change Requests', crUnreviewed, 'No un-reviewed change requests')}
131+
{renderChangeRequests('My Recently Approved Change Requests', crApproved, 'No recently approved change requests')}
128132
</Box>
129133
);
130134
};

0 commit comments

Comments
 (0)