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

#1031 Restored the Settings UI. Deferred model loading. #1046

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Conversation

kittaakos
Copy link
Contributor

Motivation

This PR will restore the (advanced) Settings UI in the IDE2. The proposed changes also defer the model (and UI) loading until the IDE2 is ready. Delaying the model load keeps the startup's performance OK and the Settings UI feature in IDE2.

Change description

How to verify:

  • Open the IDE2, and open the preferences as described in GUI-based settings commands produce an empty interface #1031,
  • Verify that the preferences UI is the same as before rc7,
  • Close the IDE2, and reopen it.
  • The widget is open, the content is unavailable, then the UI refreshes. Verify that the Settings UI feature is fully functional.
deferred_pref_model_loading.mp4

Other information

Closes #1031

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)

Closes #1031

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos kittaakos requested review from fstasi and per1234 June 11, 2022 13:43
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

This fixes #1031 for me.

However, this assertion from the test procedure provided in the PR message is failing for me:

the content is unavailable

image

For me, on both my Windows and Linux machines it switches directly from this:

image

to this:

image

I did a frame by frame check of a screen recording and it is not a matter of it just flashing through the "That search query has returned no results." quickly.

I thought I should mention this discrepancy as it might indicate that the "Deferred model loading" is not working as intended.

@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Jun 11, 2022
@kittaakos
Copy link
Contributor Author

For me, on both my Windows and Linux machines it switches directly from this:

Yes, for me too. I downloaded the bundled app and could not notice the unloaded state. The UI loaded promptly.

I looked into it, and the deferring works. Here is the proof. If I open the dev tools and set a breakpoint before updating the model, I can see the unavailable content state on the UI. It's excellent news. It looks like the IDE2 loads and runs faster in the final bundled app than from the sources.

settings_ui_before_model_update.mp4
  • The widget is open, the content is unavailable, then the UI refreshes. Verify that the Settings UI feature is fully functional.

Note: I made the initial behavior screencast by running the IDE2 from the sources.

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

My concern about the failing test assertion has been resolved, so this is a 👍 from me.

Thanks Akos!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

GUI-based settings commands produce an empty interface
2 participants