search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.4k stars 1.34k forks source link

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

Open lindapaiste opened 9 months ago

lindapaiste commented 9 months ago

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.

Keshav-0907 commented 9 months ago

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 commented 9 months ago
rahulrana701 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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

lindapaiste commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

ash97531 commented 9 months ago

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 https://github.com/processing/p5.js-web-editor/discussions/2984