Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 spec for NestedFrame.md #4950
Add spec for NestedFrame.md #4950
Changes from 4 commits
132f3f6
cc43133
44eedbe
8d67f77
6e42f58
1fd53bc
04b1b94
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at InitializeEventView and InitializeFrameEventView the functions look almost identical. Can we add a property like
CoreWebView2Frame CoreWebView2.TopLevelFrame {get;};
that is the main ICoreWebView2 object as an ICoreWebView2Frame? That would make lives easier for end devs for cases like these where they want to run the same code on the child frames and the top level CoreWebView2. In the sample code you could remove the first InitislizeEventView method and instead run InitializeFrameEventView on the CoreWebView2.TopLevelFrameThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly by our usual rules of
I{interface name}{event name}EventHandler
this would beICoreWebView2FrameFrameCreatedEventHandler
. If we think it looks weird then perhapsICoreWebView2FrameChildFrameCreatedEventHandler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a real-world sort of scenario where we would want to use this event? We want samples that demonstrates why an API is useful via a sample that an end dev could want to put in their app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC we had discussed that this addition would have impact on existing WebView2 APIs where various events can now sort of bubble from one corewebview2frame to their parent to their parent ... to the corewebview2. And how that interacts with event args for cancellation. This should probably be discussed somehow in this API spec. If it doesn't make sense to include in the API ref docs for CoreWebView2Frame.FrameCreated then perhaps you can add it to a new 'Appendix' section at the bottom that lists the other changes you plan to make to the other ref docs of impacted APIs.