Skip to content

Commit

Permalink
GitHub Deployments: Remove popup logic on installation update (#88738)
Browse files Browse the repository at this point in the history
* GitHub Deployments: Remove popup logic on installation update

* Cleanup openPopup function
  • Loading branch information
zaguiini authored Mar 20, 2024
1 parent 69734ea commit 4cda5a5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 55 deletions.
Original file line number Diff line number Diff line change
@@ -1,28 +1,23 @@
import { ExternalLink, Spinner } from '@wordpress/components';
import { useI18n } from '@wordpress/react-i18n';
import { MouseEventHandler, useLayoutEffect, useState } from 'react';
import { useLayoutEffect, useState } from 'react';
import Pagination from 'calypso/components/pagination';
import {
SortDirection,
useSort,
} from 'calypso/my-sites/github-deployments/components/sort-button/use-sort';
import { useDispatch } from 'calypso/state';
import { infoNotice } from 'calypso/state/notices/actions';
import { GitHubInstallationData } from '../../use-github-installations-query';
import {
GitHubRepositoryData,
useGithubRepositoriesQuery,
} from '../../use-github-repositories-query';
import { openPopup } from '../../utils/open-popup';
import { NoResults } from './no-results';
import { GitHubRepositoryListTable } from './repository-list-table';

import './style.scss';

const pageSize = 5;

const NOTICE_ID = 'github-app-install-notice';

interface RepositoriesListProps {
installation: GitHubInstallationData;
query: string;
Expand All @@ -38,19 +33,16 @@ export const GitHubBrowseRepositoriesList = ( {
onSelectRepository,
}: RepositoriesListProps ) => {
const { __ } = useI18n();
const dispatch = useDispatch();
const { key, direction, handleSortChange } = useSort( 'name' );
const [ page, setPage ] = useState( 1 );

useLayoutEffect( () => {
setPage( 1 );
}, [ query ] );

const {
data: repositories = [],
isLoading: isLoadingRepositories,
refetch,
} = useGithubRepositoriesQuery( installation.external_id );
const { data: repositories = [], isLoading: isLoadingRepositories } = useGithubRepositoriesQuery(
installation.external_id
);

const filteredRepositories = applySort(
filterRepositories( repositories, query ),
Expand All @@ -71,42 +63,6 @@ export const GitHubBrowseRepositoriesList = ( {
return <NoResults manageInstallationUrl={ installation.management_url } />;
}

const openPermissions: MouseEventHandler = ( event ) => {
event.preventDefault();

const openedPopup = openPopup( {
url: installation.management_url,
target: '_blank',
onMessage: ( data, popup ) => {
if ( 'github-app-installed' === data.type ) {
popup.close();

const { installationId } = data as { installationId?: number };

if ( ! installationId ) {
dispatch(
infoNotice(
__(
'Installation requested. You will be able to see it once approved by the organization owner.'
),
{
id: NOTICE_ID,
showDismiss: true,
}
)
);
}

refetch();
}
},
} );

if ( ! openedPopup ) {
window.location.href = installation.management_url;
}
};

return (
<div className="github-repositories-list">
<GitHubRepositoryListTable
Expand All @@ -124,7 +80,7 @@ export const GitHubBrowseRepositoriesList = ( {
/>
<p className="github-repositories-list-permissions-notice">
{ __( 'Missing GitHub repositories?' ) }{ ' ' }
<ExternalLink onClick={ openPermissions } href={ installation.management_url }>
<ExternalLink href={ installation.management_url }>
{ __( 'Adjust permissions on GitHub' ) }
</ExternalLink>
</p>
Expand Down
9 changes: 3 additions & 6 deletions client/my-sites/github-deployments/utils/open-popup.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
interface OpenPopupOptions {
url: string;
target?: string;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
onMessage( message: any, popup: Window ): void;
}

export const openPopup = ( { url, target, onMessage }: OpenPopupOptions ) => {
export const openPopup = ( { url, onMessage }: OpenPopupOptions ) => {
let popup: Window | null;

try {
Expand All @@ -17,10 +16,8 @@ export const openPopup = ( { url, target, onMessage }: OpenPopupOptions ) => {

popup = window.open(
url,
target,
target === '_blank'
? undefined
: `width=${ width },height=${ height },top=${ top },left=${ left }`
undefined,
`popup=1,width=${ width },height=${ height },top=${ top },left=${ left }`
);
} catch {
return false;
Expand Down

0 comments on commit 4cda5a5

Please sign in to comment.