-
-
Notifications
You must be signed in to change notification settings - Fork 488
#1580 Create remote sketch #1581
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
Conversation
05c554f to
942d974
Compare
settings.json file
#1594
fb544f9 to
1e17ed5
Compare
arduino-ide-extension/src/electron-browser/theia/core/electron-main-menu-factory.ts
Show resolved
Hide resolved
I have two requests:
- to add a shortcut to for New Remote Sketch. (
ctrl/cmd + alt/opt + Nshould be available)
- to add New Sketch and New Remote Sketch to the Command Palette, so that can be triggered with
ctrl/cmd + shift + Pand then searching for a substring, like this:
@kittaakos @91volt what do you think?
There was a problem hiding this 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
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.
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?
so we could easily update every client.
Are you aware that IDE2 generates the new remote sketch on the client?
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.
OK. Let's do that.
- 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 + Pand 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.
We do the followings:
- IDE2 does not prompt but always automatically pulls and opens the new remote sketch.
- When creating a new remote sketch from one window, the second window must be updated with the new, pulled remote sketch.
- Add keybinding to create a new remote sketch. It should be Ctrl/⌘+Alt+N.
- IDE2 features won't be exposed as commands in the context of this PR. The follow-up task is Consider exposing IDE2 features via the _Command Palette_ #1637 .
- IDE2 does not yet show the
cloudin the title bar if a remote sketch is opened. A follow-up task is tracked under Remove represented file icon from the macOS title bar #1638 .
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!
89e6776 to
91dcdb0
Compare
- When creating a new remote sketch from one window, the second window must be updated with the new, pulled remote sketch.
Please review. Thank you!
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
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
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?
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.
There was a problem hiding this 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!
arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketchbook-tree-model.ts
Outdated
Show resolved
Hide resolved
Closes #1580 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
91dcdb0 to
b381cee
Compare
Uh oh!
There was an error while loading. Please reload this page.
Motivation
To create a remote sketch from IDE2 without opening https://create.arduino.cc/editor in a browser.
Change description
File>Newhas been renamed toFile>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
Screen Shot 2022年10月31日 at 16 13 47
Advanced>Show/Hide Remote Sketchbookor"arduino.cloud.enabled": falseinsettings.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. (削除ここまで)(削除)Yeswill pull the remote sketch content and open the sketch in a new window. (削除ここまで)(削除) SelectAlwaysif you want to pull the remote sketch automatically and open it in a new window. (削除ここまで)(削除) SelectNeverto 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 newarduino.cloud.sketchOpenStrategyadvanced 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