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

Revamp of the FSBrowser and SDWebServer examples #7182

Merged
merged 138 commits into from
Apr 29, 2020
Merged

Revamp of the FSBrowser and SDWebServer examples #7182

merged 138 commits into from
Apr 29, 2020

Conversation

vdeconinck
Copy link
Contributor

@vdeconinck vdeconinck commented Apr 2, 2020

Following a discussion in issue #7002 , I proposed to unify the versions (FSBrowser and SDWebServer) and make it a "universal" FS browser. I also tried to modernize calls (e.g. using SDFS instead of SD).
One thing leading to another, I ended up improving usability and adding many features, such as a file size, free space bargraph and rename/move features.
So this is kind of "FSBrowser on steroïds". Hope you'll like it.
Please don't hesitate to test and give feedback.

Here is the hi-level changelog :

Fixes to work on LittleFS based on SDFS

  • #define logic to select FS
  • switched from SD to SDFS
  • begin() does not support parameters > removed SS and added optional config
  • LittleFS.open() second parametsr is mandatory > specified "r" where needed
  • 'FILE_WRITE' was not declared in this scope > replaced by "w"

UI/usability improvements

  • Never format filesystem, just return "FS INIT ERROR" when FS cannot be mounted
  • Tree panel width is now proportional (20%) to see long names on big screens
  • Added icons for files, and indented them to the same level as folders
  • Changed file/folder icon set to use lighter and more neutral one, and added specific "text" and "image" icons for formats recognized as such
  • Items are now sorted (folders first, then plain files, each in alphabetic order)
  • Added file size after each file name
  • Added FS status information at the top right
  • Made clear that an async operation is in progress by dimming screen and showing opertation status
  • Filled filename box in header with the name of the last clicked file
  • Selecting a file for upload defaults to putting it in the same folder as the last clicked file
  • Removed limitation to 8.3 lowercase filenames
  • Support Filenames without extension, Dirnames with extension
  • Improved recursive refresh of parts of the tree (e.g. refresh folder upon file delete, show last folder upon creating nested file)
  • Added Save/Discard/Help buttons to ACE editor, discard confirmation on leave, and refresh tree and status upon save
  • Removed "Upload" from context menu (which didn't work anyway)
  • Added "Rename/Move" feature to context menu

Other

  • Added a readme.md, as well as the PNG source for the icons and both full text and minified gz versions of the web page

LittleFS based on a #define logic (required adding a second param to open() and
replacing 'FILE_WRITE' by "w") + Added size information to file list and a /status request handler to return filesystem status
…r and

more neutral), including one for files. Show size of files. Fill
"filename" box upon clicking on a file. Sort files alphabetically.
+ Massive cleanup/merge/align with some code from the FSBrowser example
and misc refactorings
…ced FSConfig to prevent FS formating.

Fixed recursive deletion.
Got rid of specific isDir() for SPIFFS.
Refresh only part of the tree when possible.
Selecting a file for upload defaults to the same folder as the last
clicked file.
Removed the Mkdir button on SPIFFS.
Slight refactoring of XMLHttpRequest completion handling
Case insensivity of the extension for the editor and preview.
Added Save/Discard/Help buttons to Editor, discard confirmation on leave, and refresh tree/status upon save.
Removed redundant Ctrl-Z + Ctrl-Shift-Z shortcut declarations.
Small bug fixes.
+ some refactoring
…path as response to the delete request.

Refactoring
…tip.

Unsupported files on SPIFFS (files at root not sarting with "/", files with double "/", files ending with "/") are now detected and reported in the page.
Added "loading" screen during async operations (dim with spinner and status).
Fixed "discard" feature that kept prompting even after an image was loaded.
Improved refresh of parts of the tree, with recursive listing.
Moved the "path" id attribute to the "li" elements for folders (was already the case for files).
Refactoring and cleanup.
Removed non-functional Upload context menu.
Fixed error in response to move requests.
Added minified version.
Fixed incompatibilities with SPIFFS.
Fixed a race condition between deletion and reinsertion of nodes when multiple folders are refreshed.
Fixed missing URL decoding for files with special chars (e.g. space char).
Moved info from source code comment to a readme.md file.
Added source PNG to git.
Cleanup.
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather close. Some replies to the unresolved conversations.

@vdeconinck
Copy link
Contributor Author

vdeconinck commented Apr 29, 2020

Hi devyte, First, many thanks for the extensive review and advice. Much appreciated.

Rereading some comments, I realize some moves I made may seem to come out of the blue, but this version is really a merge between the original FSBrowser and SDWebServer. For example, the fact that the page is /edit/index.htm and not /edit.htm[.gz] comes from SDWebServer. Same for the recursive deletion routine.
I guess it means I really didn't invent anything, I'm mostly the cook who mixed the right ingredients :-).

With the latest requested changes (and removal of local urlDecode()), I think the version is quite OK.

With respect to the "is the server alive ?" use of /all, I just compiled a FSBrowser for LittleFS while the filesystem was formated as SPIFFS (please note that no formating ever happens anymore in such a case) and all calls return a clean "FS INIT ERROR" message. OTOH, a call to /status returns the status of the filesystem, which in that case was {"type":"LittleFS", "isOk":"false","unsupportedFiles":""}, so I think it's quite usable this way.

Please don't hesitate to test in real life and report of course.

If everything is OK, I now have two PRs on my TODO list:

  • Make the lib version of getContentType() callable from user sketches, and adapt FSBrowser accordingly
  • Create a basic example showing a live graph of ESP data.

Regarding that last point, do you still have a html page at hand that calls the graphs.js file ? I would like to know how it's meant to work before refactoring.

Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done, approved!

@devyte
Copy link
Collaborator

devyte commented Apr 29, 2020

About the two PRs, let's discuss in gitter, just @ me in the channel.

@devyte devyte merged commit 668b33d into esp8266:master Apr 29, 2020
@vdeconinck vdeconinck mentioned this pull request May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants