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

Cannot delete the first workspace while deletion icon is active #38

Closed
bramhoven opened this issue Oct 21, 2022 · 12 comments
Closed

Cannot delete the first workspace while deletion icon is active #38

bramhoven opened this issue Oct 21, 2022 · 12 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@bramhoven
Copy link
Contributor

bramhoven commented Oct 21, 2022

I don't know whether this is a bug or intended behaviour, but either way something goes wrong.

The first workspace cannot be deleted like the other workspaces while it can be renamed.

If it cannot be deleted, the trash icon should not be shown in my opinion.

@helloanoop
Copy link
Contributor

@bramhoven You cannot delete the workspace that is currently open

In order to delete it, you can switch to another workspace and the delete it.

This feedback helps. I will update the error messaging to indicate this.

The project is very early.
@bramhoven Thanks so much for trying it. Please feel free to submit issues / feature requests that bruno needs to do solve your uses cases.

@bramhoven
Copy link
Contributor Author

Ah ofcourse! Did not even notice that haha. I could also submit a PR to update the error message if you are busy with other stuff
Really like bruno so far, keep the progress going!

@helloanoop
Copy link
Contributor

@bramhoven That'd be awesome.

Here is the guide to get started and bring bruno on your local

To save you some time, here is where the generic error message is being thrown and here the error gets thrown

I have some code already baked in to implement generic error handling. Its here

The way I would approach this it throw a BrunoError in workspaces actions, and then have the DeleteWorkspace use the toastError

I also, think it'd be good to update toastError to support a default error

export const toastError = (error, defaultErrorMsg = 'An error occurred') => {
  if (error instanceof BrunoError) {
    if (error.level === 'warning') {
      return toast(error.message, {
        icon: '⚠️',
        duration: 3000
      });
    }
    return toast.error(error.message, {
      duration: 3000
    });
  }

  return toast.error(error.message || defaultError);

@helloanoop helloanoop added good first issue Good for newcomers help wanted Extra attention is needed labels Oct 21, 2022
@helloanoop helloanoop modified the milestones: v2.alpha, v1.alpha Oct 21, 2022
@bramhoven
Copy link
Contributor Author

Trying to run bruno, but getting this error.

bruno on  feature/improve-error-workspace-deletion via  v14.17.0 
❯ npm run dev:web

> usebruno@ dev:web C:\workspaces\bruno
> npm run dev --workspace=packages/bruno-app

npm ERR! missing script: dev

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\bram.hoven\AppData\Roaming\npm-cache\_logs\2022-10-21T15_01_03_235Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! usebruno@ dev:web: `npm run dev --workspace=packages/bruno-app`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the usebruno@ dev:web script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\bram.hoven\AppData\Roaming\npm-cache\_logs\2022-10-21T15_01_03_278Z-debug.log

Currently the scripts in package.json are:

"scripts": {
  "dev:web": "npm run dev --workspace=packages/bruno-app",
  "build:web": "npm run build --workspace=packages/bruno-app",
  "dev:electron": "npm run dev --workspace=packages/bruno-electron",
  "build:chrome-extension": "./scripts/build-chrome-extension.sh",
  "build:electron": "./scripts/build-electron.sh"
}

So it is missing the dev script required to run npm run dev --workspace=packages/bruno-app

@helloanoop
Copy link
Contributor

Hmm.. strange..
Can you check if the npm version is 8.x
This error is common in older npm versions that do not support workspaces

If the issue still persists, then for now, you can

cd packages/bruno-app
npm run dev

@bramhoven
Copy link
Contributor Author

I was running version 6.14. I am upgrading now. Thanks for the quick response!

@bramhoven
Copy link
Contributor Author

@helloanoop I created PR #39. If you have the time let me know what you think about it!
I also added that the error messages is gotten through the parseError method since that seemed specifically made for getting the correct message from an error.

@helloanoop
Copy link
Contributor

Merged !
Thanks for this @bramhoven !

The fix should be deployed in the web version https://play.usebruno.com/ in the next couple of min.
Will be cutting a desktop release with these fixes tomorrow.

@bramhoven
Copy link
Contributor Author

Great! I did see the Vercel build had failed... I was not able to see why it had failed though

@helloanoop
Copy link
Contributor

Yup. Some deployment config got updated in the vercel settings.
This is fixed now. The latest build is now deployed to https://play.usebruno.com

@bramhoven
Copy link
Contributor Author

Cool! Tested it and it seems to work 😁

@helloanoop
Copy link
Contributor

@bramhoven Release v0.2.0 with your fixes is out. Thanks again for the work on this.
You can download the latest versions at https://www.usebruno.com/downloads

In case you love dark more, its included in the v0.2.0 release
Currently its based on vscode dark theme, but plan to support custom themes in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants