-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
Change request reject UI #4489
Change request reject UI #4489
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
…-request-reject-ui
@@ -161,25 +156,7 @@ export const ChangeRequestOverview: FC = () => { | |||
<ChangeRequestBody> | |||
<StyledAsideBox> | |||
<ChangeRequestTimeline state={changeRequest.state} /> | |||
<ChangeRequestReviewers |
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.
moved to a new component so that adding rejectors is easier
approvals: IChangeRequestApproval[]; | ||
} | ||
|
||
export const ChangeRequestApprovals: FC<ChangeRequestApprovalProps> = ({ |
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.
approvers extracted into a new components
rejections: IChangeRequestApproval[]; | ||
} | ||
|
||
export const ChangeRequestRejections: FC<ChangeRequestRejectionProps> = ({ |
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.
rejectors just added in this PR
> | ||
{name} | ||
</Typography> | ||
<ReviewerName variant="body1">{name}</ReviewerName> |
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.
extracted styled component
condition={Boolean( | ||
uiConfig?.flags?.changeRequestReject | ||
)} | ||
show={ |
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.
this will open a modal in the next PR. For now it makes a direct change to test it E2E
{children} | ||
</Paper> | ||
); | ||
}; | ||
|
||
export const ChangeRequestReviewers: FC<{ |
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.
Existing code extracted here + added rejectors init
@@ -72,11 +72,13 @@ export const ChangeRequestsTabs = ({ | |||
const open = changeRequests.filter( | |||
changeRequest => | |||
changeRequest.state !== 'Cancelled' && | |||
changeRequest.state !== 'Rejected' && |
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.
rejections show on closed list
import { FC, useContext, useState } from 'react'; | ||
import { Box } from '@mui/material'; | ||
import { Alert, Box, Button, styled, Typography } from '@mui/material'; | ||
import React, { FC, useContext, useState } from 'react'; |
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.
React seems not needed?
@@ -0,0 +1,30 @@ | |||
import React, { FC } from 'react'; | |||
import { Typography } from '@mui/material'; | |||
import { ConditionallyRender } from '../../../common/ConditionallyRender/ConditionallyRender'; |
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 be without ../..
Not blocking approval, minor comments. |
About the changes
Important files
Discussion points