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

add new extension LibraryViewExtension (based on Microsoft WPF.WebView) #10233

Closed

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Dec 18, 2019

Purpose

replaces #10222

This PR differs in a few way:

  • adds a new extension instead of modifying the existing one
  • this one builds!
  • links some static resource files from existing LibraryViewExtension but embeds them into new dll.
  • duplicates most files from LibraryViewExtension - but adds a new namespace - Dynamo.LibraryViewExtensionMSWebView to avoid conflicts between them incase somehow these dlls are loaded at the same time by accident.

here is what I know we need to do (not all of this will be done in this PR)

  • make startup faster
  • hide the webView when startup page is visible - this is called the airGap problem if anyone is interested: WPF and Win32 interop: 'Airspace' issue dotnet/wpf#152
  • There are no tests for this implementation.
  • clean up code.... 🔨
  • eventually refactor out a common core of types from libraryJS and this extension to share. (longterm)
  • add a new .sln for this project (TBD)
  • determine how to get this signed with BRE's help or CI library and delivered to artifactory/nuget
  • this project should include a readme describing how to integrate this extension and replace the existing library.

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.

Reviewers

(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)

(FILL ME IN, optional) Any additional notes to reviewers or testers.

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

add make core internals visible
add duplicate files as required
added package references
namespace adds dynamo
use new namespace to reference resources that are embedded.
@mjkkirschner mjkkirschner added the DNM Do not merge. For PRs. label Dec 21, 2019
@mjkkirschner
Copy link
Member Author

unfortunately it looks like while this works okay in DynamoSandbox, it does not work in more complex hosts - which makes it rather useless - I will send a PR soon for a WebBrowser (IE 11 rather than Edge) based version for feedback asap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM Do not merge. For PRs. WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant