Skip to content

DYN-8121 SplashScreen Disable Close Button #15756

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

Conversation

RobertGlobant20
Copy link
Contributor

Purpose

Removing all the Dynamo code (including unit tests) that is used for the close SplashScreen functionality.

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
  • This PR contains no files larger than 50 MB

Release Notes

Removing all the Dynamo code (including unit tests) that is used for the close SplashScreen functionality.

Reviewers

@QilongTang

FYIs

Removing all the Dynamo code (including unit tests) that is used for the close SplashScreen functionality.
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8121

@RobertGlobant20
Copy link
Contributor Author

GIF showing the expected behavior in SplashScreen (close button removed).
devenv_1MK9Fi5lmT

@QilongTang QilongTang added this to the 3.5 milestone Jan 14, 2025

/// <summary>
/// [Obsolete] Constructor for ScriptObject
/// </summary>
[Obsolete]
public ScriptObject(Action<bool> requestLaunchDynamo, Action<string> requestImportSettings, Func< bool> requestSignIn, Func<bool> requestSignOut, Action requestCloseWindow)
public ScriptObject(Action<bool> requestLaunchDynamo, Action<string> requestImportSettings, Func< bool> requestSignIn, Func<bool> requestSignOut)
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 usually API breaking but I remember in the past we announced that we are not going to guard our UI code to follow backward compatible pattern. @mjkkirschner Correct me if I am wrong

@QilongTang
Copy link
Contributor

Can you also check if any of the close action code to be removed here is used in DynamoRevit repo?

@RobertGlobant20
Copy link
Contributor Author

Can you also check if any of the close action code to be removed here is used in DynamoRevit repo?

In a ECS machine with Revit 2026.0.0_20250111 and Dynamo 3.5.0 (DYN-8121-SplashScreen-Disable-CloseButton-Dynamo branch) I replaced the references DynamoCore.dll and DynamoCoreWpf.dll generated by Dynamo solution in the DynamoRevit.csproj (just in this one) and compiled successfully DynamoRevit but when trying opening SplashScreen in Revit 2026 it didn't finish loading (not sure if is due that the dll were debug or if I have to replace all the Dynamo dlls generated in all DynamoRevit projects), not sure if this is enough or if you know another way to test this please let me know. Thanks

chrome_w5TsIot7x1
.

@reddyashish
Copy link
Contributor

Changes look good but need to check if that is an API break or not.

@QilongTang
Copy link
Contributor

Please update the version of SplashScreen in this PR as well

Updating the  SplashScreen version to be consumed
@RobertGlobant20
Copy link
Contributor Author

Please update the version of SplashScreen in this PR as well

Updated in the next commit: 841d756

@QilongTang
Copy link
Contributor

Thank you @RobertGlobant20 There are some failures on the PR checks about declared APIs not found because of your removal, can you fix it? e.g. Symbol 'Dynamo.UI.Views.ScriptObject.CloseWindow() -> void' is part of the declared API, but is either not public or could not be found

Fixing deleted public API (the deleted methods were inside SplashScreen.xaml.cs then shouldn't be considered as public API).
@QilongTang
Copy link
Contributor

@avidit I bet we need your help updating one of the test closing splash screen as it used to support. For now, I will merge this one

@QilongTang QilongTang merged commit c0ebe10 into DynamoDS:master Jan 28, 2025
23 of 24 checks passed
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.

3 participants