Skip to content

Conversation

@mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Dec 14, 2019

Purpose

This PR removes CEF and CEFSharp from the libraryViewExtension and replaces it with Microsoft.Toolkit.Wpf.UI.Controls.WebView
read more here:
https://docs.microsoft.com/en-us/windows/communitytoolkit/controls/wpf-winforms/webview
which renders with edge.

This investigation was started as a way for hosts of Dynamo that have dependencies on other versions of CEF to maintain a functioning and similar user experience.

The available apis are much simpler than CEFSharp - and much more restricted - but after much finagling I think I have almost everything working as before with slower startup time, but faster actual performance during use. The code is simpler, but currently fragile, we can work to improve its robustness over time.

❗️
My intention with this PR is to get feedback and testing - I think to actually deliver this component we should make it a separate extension and test it in other integrations before pulling it into core.

Description of architecture:

  • This component only really allows interacting between C# and JS in two ways. -
    window.external.notify(string) method - which triggers an event on the c# side or - InvokeScriptAsync(strings) which invokes a method on the js side - only strings can be passed between the contexts.

  • It was not hard to refactor out the events and method calls we were using previously to a single event handler with a dumb format (functionName, params) where params are some json encoded data.

  • The biggest challenge in this integration is to deal with resource loading - unlike CEFSharp - we cannot intercept requests from the browser context and return data back - we can only serve files from specific folders - I never got this to work, and didnt' really want to serialize data on the fly just to load it back into the webcontext - instead everything(images, fonts svgs etc) is embedded into the json and html that we load or that we serialize and pass to the library functions.

  • I have refactored the resourceProviders to find the correct resources and convert them to base64 encoded strings which are then used to replace the filePaths which we passed previously. This is likely wasteful and can be optimized if we have repeated icons. We can also likely cache repeated lookups for images we have read from disk previously. None of this is done (except for the default icon because come on 😉 )

interesting facts!

  • this breaks API compatibility with LibrarieJS - BUT I don't think we should consider this extension an API in any sense anyway. I think we should contract what we consider our "API" and make it super clear - for example, how revit ships a super obvious RevitAPI.dll.

Screen Shot 2019-12-13 at 8 16 49 PM

movlib

here is what I have tested:

  • search
  • loading a package
  • creating custom node
  • placing nodes
  • scroll
  • category navigation link
  • filter
  • tooltips / hover
  • expanding/compressing / browse

here is what I know we need to do:

  • make startup faster
  • pull this out into a new extension repo
  • determine how to get this signed with BRE's help and delivered to artifactory/nuget
  • 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 and the others were very bound to the previous implementation.
  • clean up code.... 🔨
  • potentially work on getting this build going - not sure it's important as I'd like to extract it from Dynamo ASAP.

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

@QilongTang @reddyashish

FYIs

@Dewb @aparajit-pratap @Amoursol @kronz @angelowang

move to MS webView
comment out files that are not used
refactor resourceProviders so we can call them directly instead of letting CefSharp do it.
use string replacement to embed resources into html
add more js functions we need to redirect dom events to c# function calls since webView does not expose cross language objects or callbacks - only strings.
NodeItemProvider and SeachProvider both now reference IconProvider to embed images as base64 resources when they generate json -
will need to do the same with layoutSpec provider.

API broken in some places - unsure this matters in this case as short term and I do not consider this extension an API. (it's very cefsharp specifc)
simplify resource handler logic and give all providers access to icon provider to lookup icons for replacement in layout spec and types
setup search callback again
invoke updates to libary ui from ui thread
use dllprovider to lookup icons from resources folder embedded into dlls

add methods to layout spec and customization to enable replacement of iconPaths with base64 data

get around strange bug with helix3d/sharpDX startup while webView is starting - these controls need to be instantiated non concurrently. (use a task delay)

move to package references for this repo to avoid annoying build failure that doesnt actually seem to do anything... but the nuget package states its a requirment

guard against icons which have alraedy been replaced.... dont search disk for base64 or svg data as a path.

zindex does nothing, lose it.
and to replace create icon in js with base64
@mjkkirschner mjkkirschner changed the title Webviewcontrol lib Webviewcontrol lib- option2 Dec 15, 2019
@mjkkirschner mjkkirschner added the DNM Do not merge. For PRs. label Dec 18, 2019
@Dewb
Copy link
Contributor

Dewb commented Dec 19, 2019

The WebView known issues list makes me a little nervous.

To improve startup time, it seems like it would be feasible to implement a simple resource-loading mechanism through InvokeScriptAsync, would it not? But if it's not actually asynchronous (see known issues) it might not return control to the user any faster than pre-stuffing resources in as base64 strings.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants