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

IDE changes #654

Merged
merged 82 commits into from
Jun 17, 2022
Merged

IDE changes #654

merged 82 commits into from
Jun 17, 2022

Conversation

giuliomoro
Copy link
Contributor

IDE: warn if another tab opens the file and/or changes it and allow to continue as read-only in the former case.

This is not yet a full 'check if file on disk has changed' as we used to have before some 2018 refactoring, because we only guard against another tab opening the file and not against any changes that may happen on disk for other reasons. However, since we have not had any such thing for almost 3 years, it's good that it's at least partially back..
To fully restore that functionality we need to restore the mtime-based workflow that also broke around the same time.

Partly addresses #648

Screenshot 2021-01-28 at 16 47 28

Screenshot 2021-01-28 at 16 47 58

Screenshot 2021-01-28 at 16 48 14

Wording and appearance can surely be improved, but hopefully all the needed infrastructure is there

@giuliomoro giuliomoro force-pushed the fix/IDE-warn-if-open-changed branch 2 times, most recently from f52f311 to 362f35e Compare February 1, 2021 19:25
@giuliomoro giuliomoro force-pushed the fix/IDE-warn-if-open-changed branch 2 times, most recently from 1d5be8e to ffe0570 Compare February 2, 2021 19:00
@giuliomoro
Copy link
Contributor Author

Another change is the addition of a "Version Details" button. This gathers info from the board (/etc/motd and /root/update.log) and displays them here.

Screenshot 2021-02-02 at 18 40 03

Screenshot 2021-02-02 at 18 59 48

Place this text in /root/update.log to test it:

DATE=2021-02-02T18:42:26Z
FILENAME=Bela-master
METHOD=update_board
SUCCESS=true

remove some of the entries and refresh the page to see fields/change/disappear

@giuliomoro giuliomoro changed the title IDE: warn if another tab opens the file and/or changes it and allow to continue as read-only in the former case. IDE changes Feb 2, 2021
@giuliomoro giuliomoro force-pushed the fix/IDE-warn-if-open-changed branch 2 times, most recently from cc1370c to baebcdd Compare February 2, 2021 19:11
@disastrid
Copy link
Contributor

I've had a look and I like this functionality but it's introduced a new problem. Here's what I found:

  1. I open the IDE at bela.local and it loads the current open project (Tab A)
  2. I open another tab at bela.local and it loads the current open project (Tab B)
  3. Tab B loading the current open project causes Tab A to throw the "File open in another tab" popup, which is what we want.
  4. If I then return to Tab B and open up a new project file to exit read-only mode, it throws the "File open in another tab" popup despite being a new file (maybe because the URLs are the same, or because the files were the same when the tab was open and there's some persistence?)

Ideally it should check if the other tab truly does have the same file open, or if it's a new file.

In the meantime I'm going to work on:

  • How to word this so the popups are clear
  • Adding a top banner or obvious alert to READ-ONLY state tabs
  • Adding some styling to the versioning functionality to make it look and scan better

@disastrid
Copy link
Contributor

disastrid commented Apr 11, 2021

Added a number of commits that include:

  • Copy:
    • UX copy for popups for entering read-only mode and exiting read-only mode
  • Interaction:
    • Workflow for read-only mode. Removed option to opt out of read-only because I don't think it makes sense, and is too many decisions for a single dialog box.
    • A red status bar message that is added when the user enters read-only mode (in index.html)
    • Exiting read-only mode. Clicking the status message spawns a popup that offers the option to exit read-only by reloading the page, or cancel and stay in read-only.
  • Refactoring:
    • Added a function with two buttons (each with events) as well as a popup with a single button that has an event (previously the only single-button popup just has a cancel button which doesn't have an event and just closes the window)
    • Gave some functions more semantic names (popup functions, function to enter and exit read-only mode)

@giuliomoro awaiting your review

@giuliomoro
Copy link
Contributor Author

  • I open the IDE at bela.local and it loads the current open project (Tab A)
  • I open another tab at bela.local and it loads the current open project (Tab B)
  • Tab B loading the current open project causes Tab A to throw the "File open in another tab" popup, which is what we want.
  • If I then return to Tab B and open up a new project file to exit read-only mode, it throws the "File open in another tab" popup despite being a new file (maybe because the URLs are the same, or because the files were the same when the tab was open and there's some persistence?)

Currently, notifications are sent to the browser tab, which is where the state is kept. Fixing this properly would require having the server keeping track of the file currently open by each tab, keep track of which tabs have been notified and, when all the other tab close the file, notify the remaining one that they are the only ones currently owning the file. This would require a fair amount of infrastructure. The only easy way to deal with this is to remove the "File open in another tab" popup altogether. We would then keep in place only the "file edited in another tab" one, which has to be persistent and therefore works with the current way the state is kept. The usefulness of "file open in another tab" is limited with respect to the amount of work that would be needed to support it properly. This would also mean that the "read-only" mode is no longer needed at this stage.

@disastrid what do you think?

The rest of the stuff looks good to me, thanks (anyone wishing to test this should run gulp locally from IDE/frontend-dev to update the IDE/public files)

@giuliomoro
Copy link
Contributor Author

OK, just rebased this on top of dev and added a plethora of updates to it. Most stuff is not visible to the user, but it was bugging me.

File upload of multiple files was not working as expected and that's where the whole journey started.

New stuff:

  • you can upload several file at once via the button
  • new / renamed filenames/folders/projects are checked against existing ones live as you types so errors are handled from the client instead (I mean, in addition to) server.
  • the file uploading queue for drag and drop is correctly handled so you get prompted for overwrite for each already existing file
  • you get prompted for overwrite when uploading via button
  • can no longer crash the server attempting to download a folder
  • can correctly create files in subfolders (one can simply create files in subfolders (and even subfolders as needed) by using / in the file name (this used to be the case but got removed at some point)
  • you can hide the current popup via ESC
  • softer warning when opening a binary file that cannot be viewed in the IDE (i.e.: no longer popup). This could use some styling. Currently looks like this

Screenshot 2022-03-17 at 20 00 52

Under the hood:

  • refactoring
  • reducing duplicated code
  • more text out of .js and into site-text.json
  • improved performance (the whole file view is no longer re-generated every 5 seconds)
  • using async/await where it helps simplifying the code
  • more stuff went into the popup class

For ease of review, I gulped everything at once at the end. I can later rebase gulping for each commit (wouldn't take more than a couple of minutes to do that).

@giuliomoro giuliomoro force-pushed the fix/IDE-warn-if-open-changed branch 2 times, most recently from 227057b to 2f34121 Compare March 18, 2022 16:04
@roberthjack
Copy link
Contributor

Version details: excellent this is finally here.

The method of update could be reworded in the popup. via 'make update' and via 'update_board' should be renamed via IDE upload and via update script.

The date of update does not seem to be correct either. I am getting the correct month, year and time but the actual day is incorrect. Always 1st March, both for update via the IDE and update via script. Tested on a mini and regular Bela.

@giuliomoro
Copy link
Contributor Author

giuliomoro commented Apr 8, 2022

Always 1st March, both for update via the IDE and update via script. Tested on a mini and regular Bela.

Cannot reproduce. @roberthjack can you show me the content of /root/update.log on your board?

The method of update could be reworded in the popup.

There is no distinction whether the update happens via the IDE or by calling make update, so we shall leave this as-is. This is for debugging purposes only (i.e.: we need to be able to parse its meaning, the user shouldn't need to).

…ff to resize(). Note: it would be nicer to handle all of this from css directly, but a first attempt proved too complex for me

This also prevents the "this is a preview" message from showing up upon
resizing a Pd patch.
duplicated entries when the sever is rebooted.
… (including non-subfolders), appropriately resizing boxes for file names so that they never overlap with the buttons, first-level subfolders are aligned with top-level files
… single error message at the end when one or more exceed the allowed size
@giuliomoro giuliomoro force-pushed the fix/IDE-warn-if-open-changed branch from 6f685ec to 1ddbc2b Compare June 17, 2022 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants