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

Allow deletion of views via the UI #1193

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

K-Meech
Copy link
Collaborator

@K-Meech K-Meech commented Dec 1, 2024

For #1148

This PR allows deletion of views via right clicking in the MoBIE viewer and selecting Delete View. Similar to the view saving options, it allows deleting views from the 'current project' (i.e. the dataset.json) or an 'external file' (any view .json saved on the file system). The 'current project' option is currently only implemented for local projects - if you try to use it with a github / s3 project it should throw an error.

Let me know if any changes are needed! I tried it out locally with a few example projects, but it would be great if more people could test this out to check everything is working correctly 😄

@K-Meech K-Meech requested a review from tischi December 1, 2024 15:54
@tischi
Copy link
Contributor

tischi commented Dec 3, 2024

Hi @K-Meech, I don't manage to locally check out your PR.
Would you mind doing this PR from a branch within this repo rather than from your fork?

@K-Meech
Copy link
Collaborator Author

K-Meech commented Dec 3, 2024

I think you can checkout the PR locally with:

git fetch origin pull/1193/head:test-pr-1193
git checkout test-pr-1193

Does that work?

@tischi
Copy link
Contributor

tischi commented Dec 3, 2024

Yes, thanks!

@tischi
Copy link
Contributor

tischi commented Dec 3, 2024

@K-Meech I made a branch test-pr-1193 where I added an error message if the dataset.json is not found. The issue is that there are now many other ways of opening MoBIE that do not include a dataset.json.
The clean solution would be to not even show the "Delete from current project" option.
I was a bit in a rush right now. I think I would still try to implement this rather than throwing the error...

@tischi
Copy link
Contributor

tischi commented Dec 10, 2024

Hi @K-Meech,

Shall we just merge? Including my additions (see above)?

@K-Meech
Copy link
Collaborator Author

K-Meech commented Dec 10, 2024

Sounds good to me - go ahead @tischi !

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.

2 participants