Skip to content

Redirect to sandbox wizard when deleting sandbox#1313

Merged
CompuIves merged 3 commits intocodesandbox:masterfrom
pshrmn:delete-redirect
Nov 18, 2018
Merged

Redirect to sandbox wizard when deleting sandbox#1313
CompuIves merged 3 commits intocodesandbox:masterfrom
pshrmn:delete-redirect

Conversation

@pshrmn
Copy link
Copy Markdown
Contributor

@pshrmn pshrmn commented Nov 18, 2018

What kind of change does this PR introduce?

This PR changes where the user is redirected after deleting a sandbox.

Fixes #1312

What is the current behavior?

The user is redirected to the new React template (/s/new).

What is the new behavior?

The user is redirected to the sandbox wizard (/s).

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

addNotification('Sandbox deleted!', 'success'),
actions.redirectToNewSandbox,
set(props`id`, 'new'),
loadSandbox,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These two actions can be removed, correct? I'm not familiar enough with the architecture, but I believe that they are only used when loading a sandbox.

history.push('/s/new');
},
redirectToSandboxWizard() {
history.push('/s');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This might be better as a replace instead of push. Is there ever a reason that the user should be able to return to a deleted sandbox? Is there defined behavior for this?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, we should make it a replace.

Copy link
Copy Markdown
Contributor

@CompuIves CompuIves left a comment

Choose a reason for hiding this comment

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

This is a great PR, long overdue. Thanks for making it! I left one small comment, after that it's ready for merge.

history.push('/s/new');
},
redirectToSandboxWizard() {
history.push('/s');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, we should make it a replace.

@CompuIves
Copy link
Copy Markdown
Contributor

Oops, clicked approve by accident 😅.

Comment thread packages/app/src/app/store/providers/Router.js Outdated
@CompuIves
Copy link
Copy Markdown
Contributor

Thanks a lot @pshrmn!

@CompuIves CompuIves merged commit 4c7654f into codesandbox:master Nov 18, 2018
@pshrmn pshrmn deleted the delete-redirect branch November 18, 2018 23:33
@lbogdan
Copy link
Copy Markdown
Contributor

lbogdan commented Nov 19, 2018

There were still lint errors... 😟

@CompuIves
Copy link
Copy Markdown
Contributor

Fixed!

CompuIves added a commit to SaraVieira/codesandbox-client that referenced this pull request Nov 23, 2018
Redirect to sandbox wizard when deleting sandbox (codesandbox#1313)

* Redirect to sandbox wizard when deleting sandbox

* Replace location when deleting

* Trailing slash

diff --git a/packages/app/src/app/store/modules/workspace/actions.js b/packages/app/src/app/store/modules/workspace/actions.js
index f34698c..6444702 100644
--- a/packages/app/src/app/store/modules/workspace/actions.js
+++ b/packages/app/src/app/store/modules/workspace/actions.js
@@ -27,6 +27,10 @@ export function redirectToNewSandbox({ router }) {
   router.redirectToNewSandbox();
 }

+export function redirectToSandboxWizard({ router }) {
+  router.redirectToSandboxWizard();
+}
+
 export function updateSandbox({ api, state }) {
   const sandboxId = state.get('editor.currentId');
   const body = {
diff --git a/packages/app/src/app/store/modules/workspace/sequences.js b/packages/app/src/app/store/modules/workspace/sequences.js
index 3cfdbc1..17e097f 100644
--- a/packages/app/src/app/store/modules/workspace/sequences.js
+++ b/packages/app/src/app/store/modules/workspace/sequences.js
@@ -39,9 +39,7 @@ export const deleteSandbox = [
   actions.deleteSandbox,
   set(state`workspace.showDeleteSandboxModal`, false),
SaraVieira pushed a commit to SaraVieira/codesandbox-client that referenced this pull request Nov 23, 2018
* Redirect to sandbox wizard when deleting sandbox

* Replace location when deleting

* Trailing slash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redirect to template selection when deleting sandbox

3 participants