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

371 improve initialization #427

Merged
merged 13 commits into from
Jun 17, 2024
Merged

371 improve initialization #427

merged 13 commits into from
Jun 17, 2024

Conversation

pdelboca
Copy link
Member

@pdelboca pdelboca commented Jun 14, 2024

Resolves #371 (partialy)

Main Changes

  • Refactor the names of some settings to make them more intuitive.
  • Add a new basic loading/splash screen to keep track of the initialization steps.
  • Cleanup some confusing PORT and serverUrl feature.
  • Rename ensure* methods to more verbose and explicit names.

Note: I didn't put much effort in the UI/UX of the splash screen since I'm expecting changes anyway.

Notes

Our current initialization process is a little bit messy. Basically we load everything in the background, initialize the application with a loading dialog, and then disable it once a background server is detected.

The main problems with this approach are the following:

  • We do not keep track (at an application level) of which step of the initialization we are.
  • Initialization steps runs in the background detached of the window lifecycle, meaning that we cannot send messages without running into race conditions.
  • We need a process to run the main application but we need an application to display the process being run.

Suggestion

In order to have a better workflow, control and messaging of the initialization steps I'm proposing a splash window that will be displayed while the application ensure all requirements are met and the server is up. This is inspired on datasette-app loading.html approach and several other resources I read online (like this one.)

Our current initialization workflow will be:

  1. Create splash window
  2. Run initialization steps (ensurePython. ensureRequirements, etc)
  3. Send messages to the window and display progress
  4. Once initialization steps finish, trigger a finished event to load the mainWindow

Even when this PR does not address directly the issues in #371 it sets the ground to address them properly. Our former workflow was full of race conditions and parallel executions that made difficult to properly catch exceptions and displaying messages before the main window display takes place.

Demo

simplescreenrecorder-2024-06-14_15.29.13.webm

@pdelboca pdelboca requested review from roll and guergana June 14, 2024 13:57
Copy link

cloudflare-workers-and-pages bot commented Jun 17, 2024

Deploying opendataeditor with  Cloudflare Pages  Cloudflare Pages

Latest commit: 138c285
Status: ✅  Deploy successful!
Preview URL: https://fa9be496.opendataeditor.pages.dev
Branch Preview URL: https://371-improve-initialization.opendataeditor.pages.dev

View logs

@romicolman romicolman requested a review from Faithkenny June 17, 2024 08:21
@romicolman
Copy link
Collaborator

romicolman commented Jun 17, 2024

Scenario 1

The user opens the ODE FOR THE FIRST TIME:

Screen 1: The tool will display a light blue background with the name of the tool while installing components. This screen will include a message: "Loading...The first time may take a few minutes" (message to be reviewed by @Faithkenny)

If the process is successful...

Screen 2: The ODE will show a message box with a summary of what the tool can do (explore data, detect errors and publish data).

Then the user will access the main screen of the ODE

If the ODE can not run successfully..

After screen 1, the tool will show this message: "Something went wrong. Please, try again or report this problem here"

Scenario 2

Let's say the app ran successfully the first time...

The tool will display a light blue background with the name of the tool and this message: Loading...(message to be reviewed by @Faithkenny)

Then the user will access the main screen of the ODE

@pdelboca pdelboca merged commit c0f4d26 into main Jun 17, 2024
8 checks passed
@pdelboca pdelboca deleted the 371-improve-initialization branch June 17, 2024 12:35
@pdelboca pdelboca mentioned this pull request Jun 24, 2024
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.

Preparing metadata (pyproject.toml) did not run successfully
3 participants