Skip to content

Discussion: deleting an asset should remove it from projects #2979

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

Open
lindapaiste opened this issue Jan 30, 2024 · 10 comments
Open

Discussion: deleting an asset should remove it from projects #2979

lindapaiste opened this issue Jan 30, 2024 · 10 comments
Labels
Area: AWS S3 Category for AWS configuration Enhancement Improvement to an existing feature

Comments

@lindapaiste
Copy link
Collaborator

Increasing Access

It is confusing for a sketch to contain an asset which no longer exists.

Feature enhancement details

Let's say that I create a sketch and upload an image to it. Then I go to the "My Assets" page and delete that file. What should happen to the sketch?

Screenshot 2024-01-30 10 05 07

Right now we do not modify the sketch at all when the asset is deleted. The image (image.svg) will still be listed in the files menu of the project. But the file itself no longer exists, and cannot be previewed or used in the project. Trying to load the image with the p5 loadImage() function it won't work, and the error messaging is very confusing because it references the actual URL of the asset ("https://assets.editor.p5js.org/6457f440a66277001a43e859/201273ad-0121-44e1-a5b4-585ea4d9fb8e.svg") rather than the local file path ("image.svg").

Screenshot 2024-01-30 10 28 56

It also causes a bug where the sketch containing the deleted file cannot be downloaded. I fixed that bug by ignoring the deleted file, but that feels like a bandaid solution. The root of the issue is more fundamental -- why do we have projects containing files which do not exist?

I'm open to discussion on how we should handle this situation. My opinion is that when someone deletes an asset we should remove it from from the files array of any projects.

@lindapaiste lindapaiste added Area: AWS S3 Category for AWS configuration Enhancement Improvement to an existing feature labels Jan 30, 2024
@Keshav-0907
Copy link
Contributor

According to me we can also approach this by introducing a assetValidator instead of deleting the sketch because there may be situations where a user could accidentally delete a file.

The assetValidator will validate the assets required by a sketch when listing the sketches.

During sketch listing, it will validate the assets and will display a "Missing Assets" warning for sketches with missing assets.

Additionally, a Toast notification or a popup listing all missing assets, can be implemented to improve user awareness and workflow efficiency.

Screenshot 2024-01-30 at 11 10 02 PM

And if the user still wants to continue and open the sketch he/she still will be able to.

And if the sketch is opened

If the sketch is opened we can improve the way the error is being shown in the console, as @lindapaiste suggested, we should notify the user about the name of missing file.

@ash97531
Copy link

  • We should ask for a confirmation message before deleting an asset along with showing all the file names in which the asset is being used.
    - if the user confirms -> we will go with @lindapaiste 's approach i.e. after deleting an asset we should remove it from the files array of any projects.
    - if he denies -> boom, the user asset is safe along with his sketches

@rahulrana701
Copy link
Contributor

@lindapaiste I agree with @ash97531 , we should ask for a confirmation first then if the user is ok we should delete the it from the array of project. Because creating asset validator as just said by @Keshav-0907 could be messy.

@raclim
Copy link
Collaborator

raclim commented Feb 1, 2024

Agree with @ash97531 as well that showing the user a list of sketches that the asset they want to delete are in before removing it would be helpful.

@ash97531
Copy link

ash97531 commented Feb 2, 2024

i would to like to work on this issue... @raclim @lindapaiste

@lindapaiste
Copy link
Collaborator Author

We already have an "are you sure?" modal but it doesn't mention the name of the sketch. How about these 3 changes:

  1. The server should delete the asset from the files array of the project when it is deleted.
  2. Modify the text of the confirmation modal to show the name of the sketch. Right now it is always only 1 sketch. In the future an asset might be able to be used in multiple sketches.
  3. Don't require confirmation if the asset is not linked to a sketch. Right now we don't have the ability to re-use assets in other sketches so an asset which is not associated with a sketch is kind of useless. We could ask for confirmation anyways but my opinion is we don't need it in this situation.

@ash97531
Copy link

ash97531 commented Feb 2, 2024

Let me be more clear:
asset is linked: that asset is there in the sketch doesn't matter whether is being used or not.
asset is used: that asset is currently imported in one of the files.

Currently, the asset tab shows the name of Sketch to which the asset is linked.

image
So I think showing the name of Sketch in the confirmation modal is not important. Moreover, we should show the file names in which that asset is used so, that the user might remember that the asset is currently being used.

Again if an asset is not used anywhere in the sketch (not imported in any of the files), we can just directly delete that asset without confirmation i.e. opinion 3.

Although, I agree with your opinion 1 and 3.

@lindapaiste
Copy link
Collaborator Author

Moreover, we should show the file names in which that asset is used so, that the user might remember that the asset is currently being used.

I think this is smart! It's potentially not 100% accurate if the files are using string concatenation to load a file, like loadImage('image' + i + '.jpg'), but that is a very rare use case. 99% accurate should be good enough.

@ash97531
Copy link

ash97531 commented Feb 3, 2024

Yep!! This will be good enough...
I would like to work on this issue.

@ash97531
Copy link

ash97531 commented Feb 9, 2024

I am getting issues in uploading images in local setup.
I guess the issue is with my AWS setup, although I have followed all the steps given in the guide.
Can anyone help me with this... so I can work on this issue.
I have described it in #2984

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: AWS S3 Category for AWS configuration Enhancement Improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

5 participants