Being unable to delete the last file and attempting to delete the last folder causes the app to crash.

[Ri-Sharma]

What is your operating system?

Windows

Actual Behavior

The issue can be divided into two parts :-

1. Not being able to delete the last file.

file.mp4

2. Deleting the last folder causes the app to crash.

folder.mp4

Hey @lindapaiste , @raclim can you assign this issue to me as I am already working on this.

Expected Behavior

1. We should be able to delete the last file just like other files.
2. Also we should be able to delete the last folder without any problems.

Steps to reproduce

Steps:

For the first one try to delete all the files that are available.

For the second one create an folder and just some files into it, then delete all root folder files. Now if you delete the created folder the web app would crash.

[lindapaiste]

I think it’s intentional that some files like index.html should not be allowed to be deleted. Because there’s no code to run if there’s no files. Ideally we should have some sort of toast that tells the user that index.html cannot be deleted if they try to delete it.

Preventing deletion of the “last file” specifically doesn’t make a lot of sense because no files will be loaded/executed anyways if there is no index.html. The sketch preview is a webpage so it must have an index.html, which can optionally load additional .js or .css files.

It’s never intentional that the app crashes 😂. You should open the console and see what the error is there.

[lindapaiste]

The Editor component will crash if this.props.file is undefined.

checkPropTypes.js:20 Warning: Failed prop type: The prop file is marked as required in Editor, but its value is undefined.
in Editor (created by Connect(Editor))

Uncaught TypeError: Cannot read properties of undefined (reading 'fileType')
at Editor.render (index.jsx:507:1)

The above error occurred in the <Editor> component:

We can either:

1. Modify the Editor component to handle the case where this.props.file is undefined.
2. (in theory) Prevent rendering of the Editor if there is no file. In reality this won't work because the sidebar expand button is part of this Editor component.
3. Ensure that this.props.file is always defined by making sure that the user cannot delete all files.

We are attempting to do option 3 right now. You have found a loophole where the user is able to delete the final file by deleting the folder which contains it.

---

There are some additional non-fatal errors which are happening right now.

When deleting index.html

The "preview app" which is executed in the <iframe> crashes.

TypeError: Cannot read properties of undefined (reading 'content')
at _t (EmbedFrame.jsx:214:53)
at EmbedFrame.jsx:293:23

When attempting to delete the last file

I thought that it was intentional that you can't delete the last file but that might just be a bug after all! It turns out that the Redux reducer cannot handle the resetSelectedFile action if the passed id is the last id, as there is no possible file to select. This crashes the onClick event handler and prevents the deleteFile action from ever executing.

ide.js:59 Uncaught TypeError: Cannot read properties of undefined (reading 'id')
at ide.js:59:63
at index.js:8:18
at Object.resetSelectedFile (bindActionCreators.js:3:12)
at t.handleClickDelete (FileNode.jsx:158:18)

[Ri-Sharma]

@lindapaiste Yeah when deleting the last file the resetSelectedFile function throws error.

client\modules\IDE\actions\ide.js

export function resetSelectedFile(previousId) {
  return (dispatch, getState) => {
    const state = getState();
    const newId = state.files.find(
      (file) => file.name !== 'root' && file.id !== previousId
    ).id;                                                                                               
    dispatch({
      type: ActionTypes.SET_SELECTED_FILE,
      selectedFile: newId
    });
  };
}

Because of the expression in const newId we are unable to delete the last file, because whenever we try to delete the last file the state.files.find() try to find any file that is not current file and is also not root and in case of last file there is no such file so we are receiving an error here. Hence preventing the file from deleting.

But in case of deleting the 'Last Folder' the when the resetSelectedFile runs it selects the file that is inside it (in case the folder is empty the behavior will be the same as when we try to delete the 'last file') so it wont throw an here. But when the state is trying to access the file that is selected (which is already deleted when we deleted the folder) it just crashes the app.

Some Points :-

1. There is also an minor bug when we select an folder the editor goes blank.
   
   But it should just show the previous selected file. (Like in VS Code)

2. I think the above bug is related to this issue because the editor should go blank only when there is no files present. (Or we can add an placeholder)

3. I wish you a very Happy New Year.

[lindapaiste]

In order to keep the previous file open we probably need to separate what is currently one property in the state into two properties. One property that tells us which file or folder to highlight in the sidebar and other that tells us which file to open in the editor. Our SET_SELECTED_FILE action would check if the id to select is a file or a folder. If it's a file then we update both the highlighting and the editing. If it's a folder then we update the highlighting but don't change the editing. FYI at some point I want to refactor the whole files reducer using RTK and I'm imagining that selectedId would be a single property rather than having an isSelectedFile property on each file.

[SharanRP]

I'm relatively new to open source, and I believe that adding a snackbar would enhance user experience by providing clearer feedback on issues. I would like to contribute by addressing this enhancement. Could you please assign me the relevant issue? I am eager to start working on it.

[Ri-Sharma]

In order to keep the previous file open we probably need to separate what is currently one property in the state into two properties.

@lindapaiste, My suggestion is we don't need to highlight the folder when it is being clicked, we can just Toggle the show/hide children whenever an folder is being is selected.

Currently the Show/Hide folder children is only working when we click the following button, but we can make it such that it will work if we press wherever (in the folder area of course).
*Open Folder
*Close Folder

[Pankaj3112]

@lindapaiste I think the root index.html should be prevented from being deleted here and some sort of alert or toast should be displayed to the user, can you assign this issue to me?

[Ri-Sharma]

@Pankaj3112, Yeah an alert like an toast would be great but I think we have to solve the underlying issue here.

[lindapaiste]

Here is an old issue: #1070. It seems like the consensus both there and here is that the user should not be able to delete the index.html file. We can either:
A) Remove the "Delete" option from the dropdown menu
B) Keep the "Delete" option visible, but when it's clicked we show a toast explaining that index.html cannot be deleted.

[Ri-Sharma]

@lindapaiste, after some exploration, I've gathered the following insights:

1. When initiating the 'Play Sketch' action, the application locates the first .html file from the files array for execution. Considering this behavior, it might not be particularly reasonable to remove the 'Delete' option from the dropdown menu
2. We need to show the toast only when there is no .html file present.

We can tackle the above problem by updating the renderSketch function from client\modules\Preview\EmbedFrame.jsx
in a following manner,

function renderSketch() {
    const doc = iframe.current;
    if (htmlFile === undefined) {                                                    +
      dispatch(showToast('HTML File not found'));                                    +
      dispatch(stopSketch());                                                        +
      doc.src = '';                                                                  +
    }                                                                                +
    else if (isPlaying) {
      const htmlDoc = injectLocalFiles(files, htmlFile, {
        basePath,
        gridOutput,
        textOutput
      });
      const generatedHtmlFile = {
        name: 'index.html',
        content: htmlDoc
      };
      const htmlUrl = createBlobUrl(generatedHtmlFile);
      // BRO FOR SOME REASON YOU HAVE TO DO THIS TO GET IT TO WORK ON SAFARI
      setTimeout(() => {
        doc.src = htmlUrl;
      }, 0);
    } 
    else {
      doc.src = '';
    }
  }

"+" indicating addition of line