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

#1580 Create remote sketch #1581

Merged
merged 1 commit into from
Nov 10, 2022
Merged

#1580 Create remote sketch #1581

merged 1 commit into from
Nov 10, 2022

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Oct 26, 2022

Motivation

To create a remote sketch from IDE2 without opening https://create.arduino.cc/editor in a browser.

Change description

  • File > New has been renamed to File > New Sketch.

  • New menu item to create a remote sketch from IDE2: File > New Remote Sketch.
    Screen Shot 2022-10-31 at 16 09 17

    • If not signed into Arduino Cloud, the menu item is disabled.
      Screen Shot 2022-10-31 at 16 13 47
    • If the Remote Sketchbook is hidden, the menu item is not visible. (Advanced > Show/Hide Remote Sketchbook or "arduino.cloud.enabled": false in settings.json)
      Screen Shot 2022-10-31 at 16 17 44
  • Can create a new sketch from the sketchbook.

    new_local_sketch.mp4
  • When logged in to Arduino Cloud and the Remote Sketchbook is not hidden, can create a new remote sketch from the Sketchbook.

    create_remote_sketch.mp4
  • Remote sketch creation gracefully handles conflicts (HTTP 409)

    handles_conflict.mp4
  • After creating the new remote sketch, IDE2 asks if the sketch has to be pulled and opened in a new window.

    • Yes will pull the remote sketch content and open the sketch in a new window.
    • Select Always if you want to pull the remote sketch automatically and open it in a new window.
    • Select Never to never do anything automatically after the remote sketch creation. Don't worry, you can still pull your sketch manually and open it.
    • You can re-configure the behavior of the notification by tuning the new arduino.cloud.sketchOpenStrategy advanced setting. The possible values are "Ask", "Never", and "Always". The default is "Ask".
  • After creating the new remote sketch, IDE2 pulls and opens it in a new window.

  • After the remote sketch creation, IDE2 pulls the content, opens the new window, and reveals the sketch in the Remote Sketchbook widget.

    create_new_remote_and_open.mp4
  • Handles gracefully when there is a mid-air deletion before the pull and open.

    handles_deletion.mp4

Other information

Closes #1580

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@kittaakos kittaakos added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Oct 26, 2022
@kittaakos kittaakos mentioned this pull request Oct 26, 2022
4 tasks
@kittaakos kittaakos force-pushed the #1580 branch 2 times, most recently from 05c554f to 942d974 Compare October 26, 2022 16:00
@kittaakos kittaakos marked this pull request as draft October 28, 2022 15:13
@kittaakos kittaakos force-pushed the #1580 branch 6 times, most recently from fb544f9 to 1e17ed5 Compare October 31, 2022 14:23
@kittaakos kittaakos marked this pull request as ready for review October 31, 2022 16:07
@AlbyIanna
Copy link
Contributor

I have two requests:

  1. to add a shortcut to for New Remote Sketch. (ctrl/cmd + alt/opt + N should be available)

image

  1. to add New Sketch and New Remote Sketch to the Command Palette, so that can be triggered with ctrl/cmd + shift + P and then searching for a substring, like this:

image

@kittaakos @91volt what do you think?

Copy link
Contributor

@AlbyIanna AlbyIanna left a comment

Choose a reason for hiding this comment

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

When creating a new remote sketch with multiple IDE windows open, only the sketchbook in the current window is updated to show the newly created item. I need to perform a manual refresh to see the change:

Screen.Recording.2022-11-03.at.16.44.49.mov

This doesn't match the behaviour of the local sketchbook, which shows the new sketch in all the windows as soon as it is saved:

Screen.Recording.2022-11-03.at.16.46.44.mov

@kittaakos
Copy link
Contributor Author

When creating a new remote sketch with multiple IDE windows open, only the sketchbook in the current window is updated to show the newly created item. I need to perform a manual refresh to see the change:

Yes, but it's the same as with 2.0.1, if you sync one window, you do not sync the other window.

no_polling.mp4

behaviour of the local sketchbook

IDE2 can poll, but we did not plan the remote sketches with polling on purpose. Users can sync.

@AlbyIanna
Copy link
Contributor

IDE2 can poll, but we did not plan the remote sketches with polling on purpose. Users can sync.

I wasn't thinking of a polling, I thought we could make use of an event to tell every client that they should update the tree. To me it makes sense because we are aware of when a remote sketch is created from the IDE2, so we could easily update every client. I'm not suggesting to update the tree also when a remote sketch is created from the webide. @kittaakos what do you think about that?

@kittaakos
Copy link
Contributor Author

so we could easily update every client.

Are you aware that IDE2 generates the new remote sketch on the client?

@AlbyIanna
Copy link
Contributor

Are you aware that IDE2 generates the new remote sketch on the client?

I am. In order to do that we should either interact with the backend (which means creating a new service I guess) or with electron-main (which probably means making use of IPC messages).

so we could easily update every client

Maybe I used the word "easily" a little lightly here, since of course it's not just as easy as listening to a simple event. But still it shouldn't be so complicated right?

Anyway, to me we could also add this as an improvement later on if you prefer it, as it is outside the scope of this PR.

@kittaakos
Copy link
Contributor Author

OK. Let's do that.

@kittaakos kittaakos marked this pull request as draft November 3, 2022 17:00
@kittaakos
Copy link
Contributor Author

  1. to add a shortcut to for New Remote Sketch

👍 I do not have a strong preference for this. We can do it.

2. to add New Sketch and New Remote Sketch to the Command Palette, so that can be triggered with ctrl/cmd + shift + P and then searching for a substring, like this:

Probably no. IDE2 never advertised its features via the Command Palette, and I do not want to open Pandora's box in the context of this PR. Please open a follow-up if you're interested in implementing it. Just because IDE2 can have yet another way to create a sketch, it does not necessarily have to have it. One can create a sketch from a keybinding, the sketchbook, and the File menu. I think these are sufficient.

@kittaakos
Copy link
Contributor Author

We do the followings:

@kittaakos
Copy link
Contributor Author

kittaakos commented Nov 4, 2022

This doesn't match the behaviour of the local sketchbook, which shows the new sketch in all the windows as soon as it is saved:

It does not work for me.

local_sketchbook.mp4

Update:

Alberto helped me to figure out the update does not work on my env due to #1600. After fixing the invalid sketch name, IDE2 works as described in #1581 (review). Thank you!

@kittaakos kittaakos force-pushed the #1580 branch 2 times, most recently from 89e6776 to 91dcdb0 Compare November 7, 2022 09:29
@kittaakos
Copy link
Contributor Author

  • When creating a new remote sketch from one window, the second window must be updated with the new, pulled remote sketch.

#444

@kittaakos kittaakos marked this pull request as ready for review November 8, 2022 09:08
@kittaakos
Copy link
Contributor Author

Please review. Thank you!

@91volt
Copy link

91volt commented Nov 9, 2022

@kittaakos
Thank you, amazing work.

Testing it in production, the only issue I see is the missing user feedback after inserting the cloud sketch name and before the sketch is open. After naming, takes around 14 secs for the sketch to get pulled, and ~20s to open the workspace (video); a solution could be an extra step in the dialog after inserting the name, showing a spinner while the sketch get created and pulled, similarly to what we do in the firmware updater (screenshot)

@davegarthsimpson @ubidefeo @AlbyIanna @kittaakos
Do you agree on that? And in case, could we address this with another issue and consider this PR approved for its fundamental scope?

Registrazione.schermo.2022-11-09.alle.16.24.34.mov

Schermata 2022-11-09 alle 16 00 50

@kittaakos
Copy link
Contributor Author

see is the missing user feedback

It exists. The UX is precisely the same as when users manually sync the sketch. It's in the Remote Sketchbook, showing as Pulling.... But I agree it's not always clear when the tree node of the new sketch is not visible. Your screencast reveals the problem. Let's improve this.

Screen Shot 2022-11-09 at 17 16 09

create_sync.mp4

Do you agree on that?
showing a spinner while the sketch get created and pulled, similarly to what we do in the firmware updater (screenshot)

Why block the UI? Why does IDE2 not show a progress notification in the right corner? Showing definite or indefinite progress this way is known by the users: it works for lib and platform installation/uninstallation and verify/upload. Can we stick to that?

@91volt
Copy link

91volt commented Nov 10, 2022

Why block the UI? Why does IDE2 not show a progress notification in the right corner? Showing definite or indefinite progress this way is known by the users: it works for lib and platform installation/uninstallation and verify/upload. Can we stick to that?

Because the user is going to be dragged out of focus as soon as the new workspace appears, and it takes about 20s: is a bit annoying if we consider that, the user start writing a line of code, and as soon the sketch get pulled is dragged out of focus.
In any case your proposal is a viable solution and we can discuss it with @ubidefeo, but as I said before:

could we address this with another issue and consider this PR approved for its fundamental scope?

@AlbyIanna
Copy link
Contributor

a solution could be an extra step in the dialog after inserting the name, showing a spinner while the sketch get created and pulled, similarly to what we do in the firmware updater (screenshot)
Do you agree on that? And in case, could we address this with another issue and consider this PR approved for its fundamental scope?

I'd like to avoid blocking the UI if possible. Maybe we can handle it in a non-blocking notification with a loading indicator or something like that. But anyway, I agree that we can discuss this separately. This PR looks already good for me.

Copy link
Contributor

@AlbyIanna AlbyIanna left a comment

Choose a reason for hiding this comment

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

I tested it again and it works great for me. Also code is good of course.

Thank you @kittaakos for addressing my remarks!

Closes #1580

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos kittaakos merged commit 7d6a2d5 into main Nov 10, 2022
@kittaakos kittaakos deleted the #1580 branch November 10, 2022 10:12
@per1234 per1234 added the topic: cloud Related to Arduino Cloud and cloud sketches label Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cloud Related to Arduino Cloud and cloud sketches topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow creation of sketches in Remote Sketchbook
4 participants