-
Notifications
You must be signed in to change notification settings - Fork 429
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
fix: Improve the UI/UX for GitHub integrations #3907
Changes from 6 commits
bb14eba
bfca370
e6fb3d0
24bd50d
fa38d29
1b17e10
4d97821
7c6be5e
859afbe
d8de450
b34ac52
079ec0e
d314723
08e1a35
5da708f
8bbe4ab
2ee6f73
4cbace3
2d71d67
3085c69
23d0c54
3969932
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,25 +3,42 @@ import PanelSearch from './PanelSearch' | |
import Button from './base/forms/Button' | ||
import Icon from './Icon' | ||
import { | ||
useCreateExternalResourceMutation, | ||
useGetExternalResourcesQuery, | ||
useDeleteExternalResourceMutation, | ||
} from 'common/services/useExternalResource' | ||
import { ExternalResource } from 'common/types/responses' | ||
import Constants from 'common/constants' | ||
import Tooltip from './Tooltip' | ||
import MyIssuesSelect from './MyIssuesSelect' | ||
import MyGithubPullRequests from './MyPullRequestsSelect' | ||
|
||
export type ExternalResourcesTableType = { | ||
featureId: string | ||
projectId: string | ||
organisationId: string | ||
repoName: string | ||
repoOwner: string | ||
} | ||
|
||
const ExternalResourcesTable: FC<ExternalResourcesTableType> = ({ | ||
type ExternalResourceRowType = { | ||
featureId: string | ||
projectId: string | ||
externalResource: ExternalResource | ||
} | ||
|
||
type PermanentRowType = ExternalResourcesTableType | ||
|
||
type GitHubStatusType = { | ||
value: number | ||
label: string | ||
} | ||
|
||
const ExternalResourceRow: FC<ExternalResourceRowType> = ({ | ||
externalResource, | ||
featureId, | ||
projectId, | ||
}) => { | ||
const { data } = useGetExternalResourcesQuery({ | ||
feature_id: featureId, | ||
project_id: projectId, | ||
}) | ||
|
||
const [deleteExternalResource, { isSuccess: isDeleted }] = | ||
useDeleteExternalResourceMutation() | ||
|
||
|
@@ -30,6 +47,177 @@ const ExternalResourcesTable: FC<ExternalResourcesTableType> = ({ | |
toast('External resources was deleted') | ||
} | ||
}, [isDeleted]) | ||
return ( | ||
<Row className='list-item' key={externalResource?.id}> | ||
<Flex className='table-column mt-1'> | ||
<Row className='font-weight-medium'> | ||
{externalResource?.type === 'GITHUB_ISSUE' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is tied to GitHub unnecessarily and overly complex. Why not just add the following in Constants
then
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added that constant and now I'm using it where it's needed. |
||
? Constants.githubType.githubIssue | ||
: Constants.githubType.githubPR} | ||
<Button | ||
theme='text' | ||
href={`${externalResource?.url}`} | ||
target='_blank' | ||
className='fw-normal ml-1 mt-1' | ||
> | ||
<Tooltip | ||
title={ | ||
<Row> | ||
{`#${externalResource?.url.replace(/\D/g, '')}`}{' '} | ||
<div className='ml-1 mb-1'> | ||
<Icon name='open-external-link' width={14} fill='#6837fc' /> | ||
</div> | ||
</Row> | ||
} | ||
place='right' | ||
> | ||
{`${externalResource?.url}`} | ||
</Tooltip> | ||
</Button> | ||
</Row> | ||
</Flex> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<div className='font-weight-medium mb-1'> | ||
{externalResource?.metadata?.status} | ||
</div> | ||
</div> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<Button | ||
onClick={() => { | ||
deleteExternalResource({ | ||
external_resource_id: `${externalResource?.id}`, | ||
feature_id: featureId, | ||
project_id: projectId, | ||
}) | ||
}} | ||
className='btn btn-with-icon' | ||
> | ||
<Icon name='trash-2' width={20} fill='#656D7B' /> | ||
</Button> | ||
</div> | ||
</Row> | ||
) | ||
} | ||
|
||
const PermanentRow: FC<PermanentRowType> = ({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The term PermanentRow is confusing, it's not obvious what this is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed |
||
featureId, | ||
organisationId, | ||
projectId, | ||
repoName, | ||
repoOwner, | ||
}) => { | ||
const [externalResourceType, setExternalResourceType] = useState<string>('') | ||
const [featureExternalResource, setFeatureExternalResource] = | ||
useState<string>('') | ||
|
||
const [createExternalResource] = useCreateExternalResourceMutation() | ||
return ( | ||
<Row className='list-item'> | ||
<Flex className='table-column px-3'> | ||
<Select | ||
size='select-md' | ||
placeholder={'Select Type'} | ||
onChange={(v: GitHubStatusType) => setExternalResourceType(v.label)} | ||
options={[ | ||
{ id: 1, type: Constants.githubType.githubIssue }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Following above comment this could be Object.values(Constants.resourceTypes).filter((v)=>v.type==='GITHUB') There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
{ id: 2, type: Constants.githubType.githubPR }, | ||
].map((e) => { | ||
return { label: e.type, value: e.id } | ||
})} | ||
/> | ||
</Flex> | ||
<Flex className='table-column px-3'> | ||
<Flex className='ml-4'> | ||
{externalResourceType == Constants.githubType.githubIssue ? ( | ||
<MyIssuesSelect | ||
orgId={organisationId} | ||
onChange={(v) => setFeatureExternalResource(v)} | ||
repoOwner={repoOwner} | ||
repoName={repoName} | ||
/> | ||
) : externalResourceType == Constants.githubType.githubPR ? ( | ||
<MyGithubPullRequests | ||
orgId={organisationId} | ||
onChange={(v) => setFeatureExternalResource(v)} | ||
repoOwner={repoOwner} | ||
repoName={repoName} | ||
/> | ||
) : ( | ||
<></> | ||
)} | ||
</Flex> | ||
</Flex> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<Button | ||
className='text-right btn-with-icon' | ||
theme='primary' | ||
onClick={() => { | ||
createExternalResource({ | ||
body: { | ||
feature: parseInt(featureId), | ||
metadata: { status }, | ||
type: | ||
externalResourceType === Constants.githubType.githubIssue | ||
? 'GITHUB_ISSUE' | ||
: 'GITHUB_PR', | ||
url: featureExternalResource, | ||
}, | ||
feature_id: featureId, | ||
project_id: projectId, | ||
}) | ||
}} | ||
> | ||
<Icon name='plus' width={30} fill='#656D7B' /> | ||
</Button> | ||
</div> | ||
</Row> | ||
) | ||
} | ||
|
||
const ExternalResourcesTable: FC<ExternalResourcesTableType> = ({ | ||
featureId, | ||
organisationId, | ||
projectId, | ||
repoName, | ||
repoOwner, | ||
}) => { | ||
const { data } = useGetExternalResourcesQuery({ | ||
feature_id: featureId, | ||
project_id: projectId, | ||
}) | ||
|
||
const renderRowWithPermanentRow = (v: ExternalResource, index: number) => { | ||
if (index === (data?.results.length || 0) - 1) { | ||
return ( | ||
<> | ||
<ExternalResourceRow | ||
key={v.id} | ||
featureId={featureId} | ||
projectId={projectId} | ||
externalResource={v} | ||
/> | ||
<PermanentRow | ||
key='permanent-row' | ||
featureId={featureId} | ||
projectId={projectId} | ||
organisationId={organisationId} | ||
repoName={repoName} | ||
repoOwner={repoOwner} | ||
/> | ||
</> | ||
) | ||
} else { | ||
// Renderizar los elementos dinámicos normales | ||
return ( | ||
<ExternalResourceRow | ||
key={v.id} | ||
featureId={featureId} | ||
projectId={projectId} | ||
externalResource={v} | ||
/> | ||
) | ||
} | ||
} | ||
|
||
return ( | ||
<PanelSearch | ||
|
@@ -38,52 +226,13 @@ const ExternalResourcesTable: FC<ExternalResourcesTableType> = ({ | |
items={data?.results} | ||
header={ | ||
<Row className='table-header'> | ||
<Flex className='table-column px-3' style={{ 'minWidth': '280px' }}> | ||
URL | ||
</Flex> | ||
<Flex className='table-column pl-1'>Type</Flex> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<Flex className='table-column px-3'>Type</Flex> | ||
<div className='table-column text-center' style={{ width: '240px' }}> | ||
Status | ||
</div> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
Remove | ||
</div> | ||
</Row> | ||
} | ||
renderRow={(v: ExternalResource) => ( | ||
<Row className='list-item' key={v.id}> | ||
<Flex className='table-column px-3'> | ||
<Button | ||
theme='text' | ||
href={`${v.url}`} | ||
target='_blank' | ||
className='fw-normal' | ||
> | ||
<p className='fs-small'>{v.url}</p> | ||
</Button> | ||
</Flex> | ||
<Flex className='table-column px-3'> | ||
<div className='font-weight-medium mb-1'>{v.type}</div> | ||
</Flex> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<div className='font-weight-medium mb-1'>{v.status}</div> | ||
</div> | ||
<div className='table-column text-center' style={{ width: '80px' }}> | ||
<Button | ||
onClick={() => { | ||
deleteExternalResource({ | ||
external_resource_id: `${v.id}`, | ||
feature_id: featureId, | ||
project_id: projectId, | ||
}) | ||
}} | ||
className='btn btn-with-icon' | ||
> | ||
<Icon name='trash-2' width={20} fill='#656D7B' /> | ||
</Button> | ||
</div> | ||
</Row> | ||
)} | ||
renderRow={renderRowWithPermanentRow} | ||
/> | ||
) | ||
} | ||
|
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.
Im confused, does this work ? It doesnt look like the correct way to open a confirmation modal, there's no message onYes etc
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 closes the modal when you delete the installation.