Skip to content
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: Import & Export from Github causes reloading the playground even before accept this step. #1908

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions packages/playground/website/src/components/layout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,17 @@ function Modals(blueprint: Blueprint) {
) : (
''
)}

<GithubImportModal
defaultOpen={(currentModal === 'github-import-modal')}
onImported={({
url,
path,
files,
pluginOrThemeName,
contentType,
urlInformation: { owner, repo, type, pr },
}) => {
url,
path,
files,
pluginOrThemeName,
contentType,
urlInformation: { owner, repo, type, pr },
}) => {
setGithubExportValues({
repoUrl: url,
prNumber: pr?.toString(),
Expand All @@ -196,7 +198,9 @@ function Modals(blueprint: Blueprint) {
setGithubExportFiles(files);
}}
/>

<GithubExportModal
defaultOpen={(currentModal === 'github-export-modal')}
allowZipExport={
(query.get('ghexport-allow-include-zip') ?? 'yes') === 'yes'
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { MenuItem } from '@wordpress/components';
import { openModal } from '../../github/github-export-form/modal';
import { setActiveModal } from '../../lib/state/redux/slice-ui';
import { PlaygroundDispatch } from '../../lib/state/redux/store';
import { useDispatch } from 'react-redux';

interface Props {
onClose: () => void;
disabled?: boolean;
}
export function GithubExportMenuItem({ onClose, disabled }: Props) {
const dispatch: PlaygroundDispatch = useDispatch();
return (
<MenuItem
aria-label="Export WordPress theme, plugin, or wp-content directory to a GitHub repository as a Pull Request."
disabled={disabled}
onClick={() => {
openModal();
dispatch(setActiveModal('github-export-modal'));
onClose();
}}
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import { MenuItem } from '@wordpress/components';
import { openModal } from '../../github/github-import-form/modal';
import { setActiveModal } from '../../lib/state/redux/slice-ui';
import { PlaygroundDispatch } from '../../lib/state/redux/store';
import { useDispatch } from 'react-redux';

interface Props {
onClose: () => void;
disabled?: boolean;
}
export function GithubImportMenuItem({ onClose, disabled }: Props) {
const dispatch: PlaygroundDispatch = useDispatch();
return (
<MenuItem
aria-label="Import WordPress theme, plugin, or wp-content directory from a GitHub repository."
disabled={disabled}
onClick={() => {
openModal();
dispatch(setActiveModal('github-import-modal'));
onClose();
}}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,19 @@
import Modal, { defaultStyles } from '../../components/modal';
import GitHubExportForm, { GitHubExportFormProps } from './form';
import { usePlaygroundClient } from '../../lib/use-playground-client';
import { addURLState, removeURLState } from '../utils';
import { PlaygroundDispatch } from '../../lib/state/redux/store';
import { useDispatch } from 'react-redux';
import { useEffect, useState } from 'react';
import { setActiveModal } from '../../lib/state/redux/slice-ui';

const query = new URLSearchParams(window.location.search);
export const isGitHubExportModalOpen = signal(
query.get('state') === 'github-export'
Copy link
Collaborator

@adamziel adamziel Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we could get rid of this signal and initialize the activeModal redux state using the github-export query parameter. This would also:

  • Fix the linter warnings about useEffect relying on isGitHubExportModalOpen.value.
  • Make the state transitions read nicer, e.g. just dispatch(setActiveModal(null)); instead of toggleModal(false); closeModal(); dispatch(setActiveModal(null));.

For context – I introduced the signal way before we had the redux store and never got to remove it.

);

interface GithubExportModalProps {
defaultOpen?: boolean;
allowZipExport: GitHubExportFormProps['allowZipExport'];
onExported?: GitHubExportFormProps['onExported'];
initialFilesBeforeChanges?: GitHubExportFormProps['initialFilesBeforeChanges'];
Expand All @@ -18,36 +24,43 @@
export function closeModal() {
isGitHubExportModalOpen.value = false;
// Remove ?state=github-export from the URL.
const url = new URL(window.location.href);
url.searchParams.delete('state');
window.history.replaceState({}, '', url.href);
removeURLState();
}
export function openModal() {
isGitHubExportModalOpen.value = true;
// Add a ?state=github-export to the URL so that the user can refresh the page
// and still see the modal.
const url = new URL(window.location.href);
url.searchParams.set('state', 'github-export');
window.history.replaceState({}, '', url.href);
addURLState('github-export');
}
export function GithubExportModal({
defaultOpen,
onExported,
allowZipExport,
initialValues,
initialFilesBeforeChanges,
}: GithubExportModalProps) {
const dispatch: PlaygroundDispatch = useDispatch();
const playground = usePlaygroundClient();
const [isOpen, toggleOpen] = useState(defaultOpen || isGitHubExportModalOpen.value);
useEffect(() => {
toggleOpen(defaultOpen || isGitHubExportModalOpen.value);
}, [defaultOpen, isGitHubExportModalOpen.value]);

Check warning on line 47 in packages/playground/website/src/github/github-export-form/modal.tsx

View workflow job for this annotation

GitHub Actions / Lint and typecheck

React Hook useEffect has an unnecessary dependency: 'isGitHubExportModalOpen.value'. Either exclude it or remove the dependency array. Outer scope values like 'isGitHubExportModalOpen.value' aren't valid dependencies because mutating them doesn't re-render the component
const handleOnClose = () => {
toggleOpen(false);
closeModal();
dispatch(setActiveModal(null));
}
return (
<Modal
style={{
...defaultStyles,
content: { ...defaultStyles.content, width: 600 },
}}
isOpen={isGitHubExportModalOpen.value}
onRequestClose={closeModal}
isOpen={isOpen}
onRequestClose={handleOnClose}
>
<GitHubExportForm
onClose={closeModal}
onClose={handleOnClose}
onExported={onExported}
playground={playground!}
initialValues={initialValues}
Expand Down
34 changes: 23 additions & 11 deletions packages/playground/website/src/github/github-import-form/modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,61 @@
import Modal, { defaultStyles } from '../../components/modal';
import GitHubImportForm, { GitHubImportFormProps } from './form';
import { usePlaygroundClient } from '../../lib/use-playground-client';
import { addURLState, removeURLState } from '../utils';
import { useEffect, useState } from 'react';
import { setActiveModal } from '../../lib/state/redux/slice-ui';
import { PlaygroundDispatch } from '../../lib/state/redux/store';
import { useDispatch } from 'react-redux';

const query = new URLSearchParams(window.location.search);
export const isGitHubModalOpen = signal(query.get('state') === 'github-import');

interface GithubImportModalProps {
defaultOpen?: boolean;
onImported?: GitHubImportFormProps['onImported'];
}
export function closeModal() {
isGitHubModalOpen.value = false;
// Remove ?state=github-import from the URL.
const url = new URL(window.location.href);
url.searchParams.delete('state');
window.history.replaceState({}, '', url.href);
removeURLState();
}
export function openModal() {
isGitHubModalOpen.value = true;
// Add a ?state=github-import to the URL so that the user can refresh the page
// and still see the modal.
const url = new URL(window.location.href);
url.searchParams.set('state', 'github-import');
window.history.replaceState({}, '', url.href);
addURLState('github-import');
}
export function GithubImportModal({ onImported }: GithubImportModalProps) {
export function GithubImportModal({ defaultOpen, onImported }: GithubImportModalProps) {
const dispatch: PlaygroundDispatch = useDispatch();
const playground = usePlaygroundClient();
const [isOpen, toggleOpen] = useState(defaultOpen || isGitHubModalOpen.value);
useEffect(() => {
toggleOpen(defaultOpen || isGitHubModalOpen.value);
}, [defaultOpen, isGitHubModalOpen.value]);

Check warning on line 35 in packages/playground/website/src/github/github-import-form/modal.tsx

View workflow job for this annotation

GitHub Actions / Lint and typecheck

React Hook useEffect has an unnecessary dependency: 'isGitHubModalOpen.value'. Either exclude it or remove the dependency array. Outer scope values like 'isGitHubModalOpen.value' aren't valid dependencies because mutating them doesn't re-render the component
const handleOnClose = () => {
toggleOpen(false);
closeModal();
dispatch(setActiveModal(null));
}
return (
<Modal
style={{
...defaultStyles,
content: { ...defaultStyles.content, width: 600 },
}}
isOpen={isGitHubModalOpen.value}
onRequestClose={closeModal}
isOpen={isOpen}
onRequestClose={handleOnClose}
>
<GitHubImportForm
playground={playground!}
onClose={closeModal}
onClose={handleOnClose}
onImported={(details) => {
playground!.goTo('/');
// eslint-disable-next-line no-alert
alert(
'Import finished! Your Playground site has been updated.'
);
closeModal();
handleOnClose();
onImported?.(details);
}}
/>
Expand Down
15 changes: 15 additions & 0 deletions packages/playground/website/src/github/utils.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

export function removeURLState() {
// Remove ?state=github-xxx from the URL.
const url = new URL(window.location.href);
url.searchParams.delete('state');
window.history.pushState({}, '', url.toString());
}

export function addURLState(name: string) {
// Add a ?state=github-xxx to the URL so that the user can refresh the page
// and still see the modal.
const url = new URL(window.location.href);
url.searchParams.set('state', name);
window.history.pushState({}, '', url.toString());
}
Loading