Revert "DYN-9702 templates to home screen (#16932)"#17013
Revert "DYN-9702 templates to home screen (#16932)"#17013zeusongit merged 2 commits intoDynamoDS:RC4.1.0_masterfrom
Conversation
This reverts commit 5dbb791.
There was a problem hiding this comment.
Pull request overview
Reverts the incomplete backend support for exposing/opening Dynamo template files on the Home Page (introduced in #16932).
Changes:
- Removes template-file collection/loading from
StartPageViewModel. - Removes template data plumbing from
HomePageinitialization and WebView script dispatch. - Removes the unshipped public API entry for
StartPageViewModel.TemplateFiles.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/DynamoCoreWpf/Views/HomePage/HomePage.xaml.cs | Removes template-data sending and adjusts recent/sample graph dispatch to the Home Page WebView. |
| src/DynamoCoreWpf/PublicAPI.Unshipped.txt | Removes the unshipped TemplateFiles getter API entry. |
| src/DynamoCoreWpf/DynamoCoreWpf.csproj | Updates compile item list (currently introduces duplicate <Compile Include=...> entries). |
| src/DynamoCoreWpf/Controls/StartPage.xaml.cs | Removes template file list/state and associated initialization logic from the start page VM. |
Comments suppressed due to low confidence (1)
src/DynamoCoreWpf/Views/HomePage/HomePage.xaml.cs:329
SendRecentGraphsDatawas changed toasync voideven though it’s not an event handler. This can crash the UI thread on exceptions fromExecuteScriptAsync, and the caller can’t observe completion/failure. Prefer restoringasync Task SendRecentGraphsData()and eitherawaitit from an async call chain or add explicit fault logging when firing-and-forgetting.
private async void SendRecentGraphsData()
{
// Send user preferences
if (dynWebView?.CoreWebView2 != null)
{
if (startPage.DynamoViewModel.PreferenceSettings.HomePageSettings != null)
{
var dict = GetDictFromListStrings(startPage.DynamoViewModel.PreferenceSettings.HomePageSettings);
var settingsJson = Newtonsoft.Json.JsonConvert.SerializeObject(dict);
settingsJson = System.Web.HttpUtility.JavaScriptStringEncode(settingsJson);
await dynWebView.CoreWebView2.ExecuteScriptAsync(@$"window.setHomePageSettings('{settingsJson}');");
| @@ -302,26 +300,10 @@ private async Task LoadGraphs(List<StartPageListItem> data) | |||
| } | |||
There was a problem hiding this comment.
LoadGraphs was changed from async Task to async void. Since this isn’t an event handler, async void exceptions propagate to the WPF synchronization context and can crash the process (e.g., if ExecuteScriptAsync faults). Prefer restoring a Task return type and either await it from an async caller or keep fire-and-forget with explicit exception handling/logging.
| @@ -333,21 +315,9 @@ private async Task SendSamplesData() | |||
| } | |||
There was a problem hiding this comment.
SendSamplesData was changed to async void but it’s not an event handler. Exceptions from ExecuteScriptAsync will be raised on the WPF sync context (potentially crashing the app) and can’t be observed/handled by the caller. Prefer restoring async Task and either await it from an async caller or keep fire-and-forget with explicit exception handling/logging.
| <Compile Include="Views\HomePage\HomePage.xaml.cs" /> | ||
| <Compile Include="Views\Input\ParameterEditor.cs" /> | ||
| <Compile Include="Views\PackageManager\Components\PackageManagerWizard\PackageManagerWizard.xaml.cs" /> |
There was a problem hiding this comment.
HomePage.xaml.cs and PackageManagerWizard.xaml.cs are now included twice in the project (also present later with <DependentUpon>). Duplicate <Compile Include=...> items typically cause MSBuild errors (duplicate items) and should be removed—keep only the existing entries that include DependentUpon.
| <Compile Include="Views\HomePage\HomePage.xaml.cs" /> | |
| <Compile Include="Views\Input\ParameterEditor.cs" /> | |
| <Compile Include="Views\PackageManager\Components\PackageManagerWizard\PackageManagerWizard.xaml.cs" /> | |
| <Compile Include="Views\Input\ParameterEditor.cs" /> |
|
@zeusongit @johnpierson Please also consider if there is any PR in DynamoHome repo need to be reverted, or if better, revert the js package version as part of this PR |
yes here is the PR for DynamoHome: DynamoDS/DynamoHome#79 Reverted DynamoHome to v26 |
|
|
LGTM |
Purpose
Reverts #16932
The work is incomplete. We will be following up with Jira tasks to round out this work.
We’ll regroup and reintroduce the changes once the work is ready in 4.2.
Declarations
Check these if you believe they are true
Release Notes
Reverts #16932