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

Save backend type in localStorage, and automatically open projects when ready #6728

Merged
merged 28 commits into from
Jun 6, 2023

Conversation

somebody1234
Copy link
Contributor

@somebody1234 somebody1234 commented May 17, 2023

Pull Request Description

Important Notes

Needs a very thorough QA. There are quite a lot of changes to the state logic, so it'll take a little time to test them all:

  • -startup.project + the wrong backend
  • canceling project creation on local backend, and canceling auto-open when you decide to open another project (either through play button or through context menu)

Screencast

screen-recorder-wed-may-17-2023-19-03-22.mp4

Note: This recording does show a few issues:

  • The project name is not changed, which breaks all projects after the first one with error "Module 'local.Name_Of_The_First_Project.Main' not found". This seems to be an issue with the Project Manager, quick debugging says that
    • Upon further thinking... it's because the command-line's startup.project overrides the one passed by the dashboard. This will be fixed in the next commit(s).
  • There are issues with getting the resources check to run consistently.
  • Sometimes, saving the current backend type does not work. The cause is unclear.
    If anyone can reliably reproduce these problems, or can figure out the cause, that would be great. I'll see if anything can be done to simplify the state logic though (and whether the simplifications will remove the issue)

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@somebody1234 somebody1234 added the CI: No changelog needed Do not require a changelog entry for this PR. label May 17, 2023
@somebody1234
Copy link
Contributor Author

ok re: removing startup.project, the code may need to be updated to work with the auth flow changes in #6668 once that is merged

@somebody1234
Copy link
Contributor Author

note: I don't think swapping the order (URL params before explicitly passed options) is a good idea, which is why it's not being considered as a potential solution here. startup.project should be immediately removed, because it is only intended as a shortcut

@somebody1234
Copy link
Contributor Author

The first two mentioned issues should now be fixed. Haven't been able to repro the third issue for a while though

@somebody1234 somebody1234 marked this pull request as ready for review May 18, 2023 07:23
@PabloBuchu
Copy link
Contributor

PabloBuchu commented May 18, 2023

First run after the build results in weird error while connecting to project manager. It's always first run the nexts are clean

Screenshot 2023-05-18 at 13 48 25

@PabloBuchu
Copy link
Contributor

When using local backend and user clicks play icon he/she is immediately taken to workspace (no need for double click to spin and open project).

This isn't working

@somebody1234
Copy link
Contributor Author

somebody1234 commented May 29, 2023

QA:

./run ide watch:
✔️ log in
✔️ play to immediately open
✔️ double click to immediately open
✔️ right click -> "open for editing" to immediately open
✔️ registration flow works - previously, ❌ passwords were not required in the initial sign up screen
✔️ change password flow works - previously ❓ ugly error from server for certain values of "old password"
✔️ user menu - previously:

  • ❓ disabled option "your profile" looked like it was enabled
  • ❓ clicking the user icon after clicking outside to unset the modal, does nothing - this is because isUserMenuVisible is still true

✔️ search works - now made case-insensitive
✔️ creating project works (local backend) - both empty, and from all templates
✔️ deleting project works (local backend)
✔️ renaming project works (local backend) - validation too
✔️ deleting project works (local backend)
✔️ creating project works (cloud backend)

  • ⚠️ project order is very wonky

✔️ renaming project works (cloud backend)
✔️ table looks ok - previously ❌ was full-width. for some reason width: 0 (w-0) is needed
✔️ persisting backend type works - previously ❌ it was overwritten to remote every refresh
✔️ search bar looks ok - previously ❓ was the default size="20" so it would push the help chat and user profile offscreen at around ~740px viewport width
✔️ templates look ok - adjusted so the tiles are a fixed size (and still 5 columns at 1280px width)
✔️ document.body scrollbar now uses the same scrollbar as visualizations - previously ❓ default scrollbar
✔️ folder navigation + path saving both work (cloud backend)
✔️ shared with is still hidden on local backend
✔️ last modified date is still visible
⚠️ many icons are still using placeholders because they did not exist on the figma design - this is definitely way out of scope, but still worth noting

✔️ can go back to dashboard from -startup.project - previously ❌ broken
✔️ invalid -startup.project now displays an error - previously ❌ did not exist
✔️ -startup.entry (with and without -startup.project, even though it does nothing)
✔️ -startup.project works when selecting ide entry point after passing invalid -startup.entry
✔️ -startup.project works with gui watch

(QA not 100% thorough but should be more than enough for one PR?)

logins:
✔️ logins work on gui watch (google, github, password)
✔️ logins work on watch-dashboard (google, github, password) - previously ❓ redirected to cloud.enso.org - was intentional, however might as well take advantage of the build tooling to override it now that it exists
✔️ logins work on ide watch (google, github, password) - ⚠️ this logs in through the builtin browser
❓ logins work on ide build - cannot test as i have neither a windows nor a mac machine handy

@somebody1234
Copy link
Contributor Author

somebody1234 commented May 31, 2023

⚠️ there's an issue with the spinner starting at 75% instead of 5% after certain actions
⚠️ there's also issues related to opening multiple projects at once (it tries to auto-start multiple IDE instances at once)

  • well, it's probably more related to cancelling autostart not working properly. if you click play and then click stop right after, the IDE will still launch

@somebody1234 somebody1234 force-pushed the wip/sb/save-backend-type branch from ae7b693 to 50cc4cf Compare May 31, 2023 19:50
@somebody1234 somebody1234 mentioned this pull request Jun 1, 2023
5 tasks
@somebody1234
Copy link
Contributor Author

somebody1234 commented Jun 2, 2023

just merged and am doing a bit of testing before pushing - it turns out there's an issue with trying to delete the project before it's closed - what should we do? i think the main issue is that the spinner is currently not set up to spin on closing - which isn't a bad thing, i think it would be a little confusing

however this means that this issue doesn't really have a good solution (that i can think of, at least)

also ⚠️ creating a cloud project and immediately deleting it causes issues:

  • the first time, it went back to the loading stage
  • the second and third times, it didn't go back to the loading stage but the toast notification stayed around forever

might be fixed now but not 100% sure

@PabloBuchu
Copy link
Contributor

@somebody1234
| it turns out there's an issue with trying to delete the project before it's closed - what should we do?

We should grey out (disable) the Delete option in context menu and add small tooltip on hover "can't remove running project"
Screenshot 2023-06-05 at 10 06 09

@PabloBuchu PabloBuchu merged commit 5cc2100 into develop Jun 6, 2023
@PabloBuchu PabloBuchu deleted the wip/sb/save-backend-type branch June 6, 2023 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--startup.project option does not work with the new dashboard.
2 participants