yonatanmgr / mathberet

Mathberet is a self-hosted digital mathematics notebook written in React and Typescript, designed for math students who need a platform for graphing, sketching, and writing in LaTeX.
MIT License
177 stars 12 forks source link

Fixed several bugs (including #4) #24

Closed zivnadel closed 1 year ago

zivnadel commented 1 year ago

I have fixed several bugs in this Pull Request.

Firstly, I addressed issue #4. I fixed it by doing something similar to what @alex-laycalvert suggested there. I changed if(focusedItem) to if(focusedItem != -1) in generateStateWithNewFolder and generateStateWithNewFolder (located in FileSystemHelpers.ts). In my opinion, this change is more accurate because item.index will either be -1 or a string. Using if(focusedItem >= 0) would work but is less clear.

Secondly, I addressed some bugs related to PR #22 regarding focusing a file. Since in that PR I changed the structure of the file system item (making the index and the object key the path of the file) it broke some stuff:

Important note: I have not yet adapted the handleRenameItem function (located in FileSystem.tsx) to the changes in PR #22. Therefore, renaming will likely be broken when it is implemented. I will make sure to fix it as soon as possible.

I believe these changes have resolved all known issues. However, if any further bugs are discovered, I will be sure to fix them as soon as possible.

zivnadel commented 1 year ago

I’ll Look into it.

zivnadel commented 1 year ago

I have fixed the two bugs that you mentioned. The first one was caused by a typo I made in the generateStateWithNewFolder function, where I mistakenly replaced newFolderName with newFileName.

The second bug was caused by the checkIfItemNameIsFolder function, which checked for the existence of files with a certain name throughout the system. I have now modified it to check for items by the provided name under a passed node (path) - in our use case, the focused item. Although I believe that this solution works well for now, we should consider passing the parent node when a folder is opened, not just when it is focused. If this indicator has already been implemented, please let me know.

zivnadel commented 1 year ago

@yonatanmgr I fixed the last bugs, and made some changes. The main change I did is that generateStateWithNewFile, generateStateWithNewFolder and itemExistsInParent (which is the new version of checkIfItemNameIsFolder, more about that below) now relate to the selectedFolder and not focusedItem. Other changes I made: