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

DYN-5268 integrate web splash screen to Sandbox #13395

Merged

Conversation

filipeotero
Copy link
Contributor

@filipeotero filipeotero commented Oct 11, 2022

Purpose

This PR contains the implementation of the splash screen

  • Loading bar containing the loading time
  • Localized labels via js function
  • Insertion of a background for the splash screen
  • Settings importations with a feedback status
  • npm install SplashScreen js package command added in the sandbox csproj to update the js bundle properly

TODO

  • Check why the dynamo UI is getting blocked after being launched
    • It has something to do with the Background Preview
  • Check why splash screen is blank. Addressed by @RobertGlobant20 DYN-5268-SplashScreen-NotLoadingFix SplashScreen#13
  • Move splash screen to DynamoCoreWpf so it is shared implementation between DynamoSandbox and DynamoView from integration
  • Initial list of loading tasks when launching Dynamo: Loading ASM, Preferences, Node Library, Dynamo View, View extensions
  • Disabled context menu on Splash Screen and fixed crashes
  • Fixed the bug about Do not Show Splash Screen again not working properly
  • Added setting in prefrences panel for user

SplashScreen

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

FYIs

@RobertGlobant20 @jesusalvino @QilongTang @zeusongit @reddyashish

src/DynamoApplications/StartupUtils.cs Outdated Show resolved Hide resolved
src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs Outdated Show resolved Hide resolved
src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs Outdated Show resolved Hide resolved
src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs Outdated Show resolved Hide resolved
src/DynamoCoreWpf/Views/Core/DynamoView.xaml.cs Outdated Show resolved Hide resolved
migrationWindow.webView.NavigationCompleted += WebView_NavigationCompleted;
migrationWindow.RequestLaunchDynamo = LaunchDynamo;
migrationWindow.RequestImportSettings = ImportSettings;
migrationWindow.Show();
Copy link
Contributor

@QilongTang QilongTang Oct 13, 2022

Choose a reason for hiding this comment

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

@filipeotero @reddyashish @zeusongit @RobertGlobant20 I notice that we may not have time to adjust this before @filipeotero 's vacation, but I think we need to rename this view and move it back to DynamoCoreWpf project instead of maintaining it as part of Sandbox project. The reason I kept wondering is in order to have the splash screen to appear in DynamoRevit context, because I think the sandbox code is not called during in-process integration case

$"welcomeToDynamoTitle: '{Properties.Resources.SplashScreenWelcomeToDynamo}'," +
$"signInTitle: '{Properties.Resources.SplashScreenSignIn}'," +
$"launchTitle: '{Properties.Resources.SplashScreenLaunchTitle}'," +
$"showScreenAgainLabel: '{Properties.Resources.SplashScreenShowScreenAgainLabel}'" + "})");
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

}
}

private void LoadDynamoView()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great but I anticipate us moving this code to DynamoCoreWpf so that this can be shared between sandbox and integrations.. I would like to discuss with you tomorrow about it

@QilongTang QilongTang marked this pull request as ready for review October 20, 2022 15:56
@QilongTang QilongTang added this to the 2.17.0 milestone Oct 20, 2022
@QilongTang QilongTang changed the title [DRAFT] DYN-5268 integrate web splash screen DYN-5268 integrate web splash screen Oct 20, 2022
@QilongTang QilongTang changed the title DYN-5268 integrate web splash screen DYN-5268 integrate web splash screen to Sandbox Oct 20, 2022
@QilongTang
Copy link
Contributor

@reddyashish @zeusongit @RobertGlobant20 @Amoursol I think I worked on this PR enough so that this is ready to merge so we can test splash screen tomorrow. There is one follow up step which I did not have time to do - to move the splash screen to Dynamo View for integrations which I can do after our bug bash tomorrow. I would like to hear your comments on this, although I would like to merge tonight for our daily build tomorrow, but comments will be addressed later in another PR

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 left a comment

Choose a reason for hiding this comment

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

LGTM

@QilongTang QilongTang merged commit bf9affb into DynamoDS:master Oct 21, 2022
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.

4 participants