Create FindOnPage.md #4399
Create FindOnPage.md #4399
Conversation
removed event handler for find operation completed handler
deleted duplicates
Deleted 3 event handlers as they did not serve an explicit purpose.
Added event handlers for match count and match index back
Added more to description
Added C# defintion of API
consolidate examples
removed c# section
updated midl3 definition
FindOnPage.md
Outdated
| /// To change the ongoing find session, StartFindAsync must be called again with a new or modified configuration object. | ||
| /// StartFind supports, HTML, PDF, and TXT document queries. In general this api is designed for text based find sessions. | ||
| //// If you start a find session programmatically on another file format that doesnt have text fields, the find session will try to execute but will fail to find any matches. (It will silently fail) | ||
| /// Note: The asynchronous action completes when the UI has been displayed with the find term in the UI bar, and the matches have populated on the counter on the find bar. |
There was a problem hiding this comment.
Is the completion of the IAsyncAction the point at which calls to Next()/Previous()/Stop() methods become allowed? What happens if I call Next() before this IAsyncAction completes, is it the same "silently fail" as when there's no active session?
There was a problem hiding this comment.
Is the completion of the IAsyncAction the point at which calls to Next()/Previous()/Stop() methods become allowed?
Yes
What happens if I call Next() before this IAsyncAction completes, is it the same "silently fail" as when there's no active session?
Yes
Please make sure this is documented in the section about Start and what its async completion means.
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
Are we pre-committed to using "Find" as both a noun and a verb in this API, e.g. because of industry standards or existing webview2 code?
"Find" is commonly a verb in API method names.
Here "Find" is will be a noun twice in coreWebView2.Find.StartFindAsync(..., and a noun and verb once each in coreWebView2.Find.FindNext().
There was a problem hiding this comment.
I also find the naming awkward here. I think we should consider something like FindOperation or FindSession over Find.
There was a problem hiding this comment.
The old API (Microsoft internal link) called it a "find session", initiated by a "find request".
There was a problem hiding this comment.
I think part of the awkwardness is usage of the word "find" rather than "search"; "search" as both a noun and a verb is more common, at least in en-us. (But "find" is the right name for this feature)
There was a problem hiding this comment.
Please update:
- CoreWebView2.Find -> CoreWebView2.FindSession
- StartFindAsync -> StartAsync
- StopFind -> Stop
(but leave FindNext/Previous as is with Find)
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
Should "Find" be consistently capitalized in this document when it's a noun?
There was a problem hiding this comment.
Please fix after renaming property name its now FindSession (including casing) throughout doc.
FindOnPage.md
Outdated
|
|
||
| ```cpp | ||
|
|
||
| //! [InitializeFindConfiguration] |
There was a problem hiding this comment.
Each example has a comment with this attribute-like syntax. Is this intended to signify something?
There was a problem hiding this comment.
I think it's used to extract the example code into a doc somewhere
There was a problem hiding this comment.
Please remove from doc.
| ### Setting Up Find Configuration with MIDL3 | ||
|
|
||
| ### CoreWebView2 Find Configuration and Direction | ||
|
|
There was a problem hiding this comment.
Are these sections missing? Or can these headings be removed?
FindOnPage.md
Outdated
| CoreWebView2FindConfiguration CreateFindConfiguration(); | ||
| }; | ||
|
|
||
| runtimeclass CoreWebView2FindConfiguration : [default]ICoreWebView2FindConfiguration {} |
FindOnPage.md
Outdated
| CoreWebView2FindConfiguration CreateFindConfiguration(); | ||
| }; | ||
|
|
||
| runtimeclass CoreWebView2FindConfiguration : [default]ICoreWebView2FindConfiguration {} |
There was a problem hiding this comment.
Since this is midl 3, if the only implementation of the interface is this runtimeclass, you can just define the runtimeclass and the matching interface will be defined implicitly.
There was a problem hiding this comment.
Same comment with regards to CoreWebView2Find/ICoreWebView2Find.
There was a problem hiding this comment.
Please validate that the generated MIDL3 doesn't have this interface as public.
FindOnPage.md
Outdated
| ```cpp | ||
|
|
||
| //! [InitializeFindConfiguration] | ||
| wil::com_ptr<ICoreWebView2FindConfiguration> AppWindow::InitializeFindConfiguration(const std::wstring& findTerm) |
There was a problem hiding this comment.
I assume that AppWindow here just refers to the class that this sample code belongs to? I initially thought it had something to do with the Microsoft.UI.Windowing.AppWindow class in WinAppSDK. Consider renaming to something else to avoid confusion.
There was a problem hiding this comment.
Please follow up with me to ensure we open a bug to change this in our sample app code.
|
|
||
|
|
||
| /// Adds an event handler for the `ActiveMatchIndexChanged` event. | ||
| /// Registers an event handler for the ActiveMatchIndexChanged event. This event is raised when the index of the currently active match changes. This can happen when the user navigates to a different match or when the active match is changed programmatically. The parameter is the event handler to be added. Returns a token representing the added event handler. This token can be used to unregister the event handler. |
There was a problem hiding this comment.
Add line breaks to keep the width of the lines at a reasonable length, e.g. 80 chars or 120 chars.
There was a problem hiding this comment.
Similarly with the rest of the long lines in this section.
| /// Displays the Find bar and starts the find operation. If a find session was already ongoing, it will be stopped and replaced with this new instance. | ||
| /// If called with an empty string, the Find bar is displayed but no finding occurs. Changing the configuration object after initiation won't affect the ongoing find session. | ||
| /// To change the ongoing find session, StartFind must be called again with a new or modified configuration object. | ||
| /// This method is primarily designed for HTML document queries. |
There was a problem hiding this comment.
This method is primarily designed for HTML document queries.
I don't understand what this comment means.
There was a problem hiding this comment.
Please fix.
This comment is out of date. The comment on the MIDL3 StartFindAsync is more up to date.
|
|
||
| /// Navigates to the next match in the document. | ||
| // MSOWNERS: core ([email protected]) | ||
| HRESULT FindNext( |
There was a problem hiding this comment.
If the FindConfiguration FindDirection is set to Backwards, FindNext will find the match that occurs before the current match - is that correct?
We should explicitly document the relation between FindNext/FindPrevious and FindDirection.
There was a problem hiding this comment.
Correct - Please explicitly state that in the ref docs for FindNext/Previous.
FindOnPage.md
Outdated
| CHECK_FAILURE(findConfiguration->put_FindTerm(findTerm.c_str())); | ||
| CHECK_FAILURE(findConfiguration->put_IsCaseSensitive(false)); | ||
| CHECK_FAILURE(findConfiguration->put_ShouldMatchWord(false)); | ||
| CHECK_FAILURE(findConfiguration->put_FindDirection(COREWEBVIEW2_FIND_DIRECTION_FORWARD)); |
There was a problem hiding this comment.
I don't know what the default find UI looks like. Does it have UI to control these options? E.g. is there a TextBox for the find term and checkboxes for case-sensitive/match-word?
If so, what does interacting with this UI do?
Similarly, is there a 'next' button? If so, what does clicking on it do?
In general, it would be good to describe what using this API with the default find UI looks like. I can understand how using this API would work in the case where I am suppressing the default UI. But in the case where I am relying on the default UI, it's unclear what I would do with this API. What is the scenario that is trying to be achieved?
There was a problem hiding this comment.
See above to add screenshot and more details on the scenario.
FindOnPage.md
Outdated
| ```cpp | ||
|
|
||
| /// Specifies the direction of find Parameters. | ||
| // MSOWNERS: core ([email protected]) |
There was a problem hiding this comment.
We don't normally include the MSOWNERS comments in the public doc
There was a problem hiding this comment.
Please fix (throughout)
FindOnPage.md
Outdated
| [propget] HRESULT FindDirection([out, retval] COREWEBVIEW2_FIND_DIRECTION* value); | ||
|
|
||
|
|
||
| /// |
There was a problem hiding this comment.
Missing descriptions for setters
There was a problem hiding this comment.
If this is generated by our tooling, please follow up with me/Jessica about generating ref docs for these.
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
The old API (Microsoft internal link) called it a "find session", initiated by a "find request".
FindOnPage.md
Outdated
| This method allows setting the find term and find parameters via the | ||
| `ICoreWebView2FindConfiguration` interface. Only one find session can be active per | ||
| webview environment. Starting another with the same configuration will adjust | ||
| the active match index based on the selected Find Direction. |
There was a problem hiding this comment.
What about starting another find session with a different configuration? Does that implicitly cancel the previous find session?
There was a problem hiding this comment.
I read this as really this always starts a Find session with the first match active. You get the same result whether or not you had the same find already running. Took me a minute to figure out, but if that's what it means, then the behavior sounds right
There was a problem hiding this comment.
The current behavior is confusing. Please ensure that calling StartFindAsync always cancels the existing find and starts a new one from scratch as if you had only just called StartFindAsync without an existing find already going on.
FindOnPage.md
Outdated
|
|
||
| // Query for the ICoreWebView2_17 interface to access the Find feature. | ||
| auto webView2_17 = m_webView.try_query<ICoreWebView2_17>(); | ||
| CHECK_FEATURE_RETURN(webView2_17); |
There was a problem hiding this comment.
Since this is just a feature check (we throw the QI away), shouldn't this be done first, before we ask for ICoreWebView2Environment5?
There was a problem hiding this comment.
Please fix by moving to top of function.
| { | ||
| // Query for the ICoreWebView2Environment5 interface. | ||
| auto webView2Environment5 = m_webViewEnvironment.try_query<ICoreWebView2Environment5>(); | ||
| CHECK_FEATURE_RETURN(webView2Environment5); |
There was a problem hiding this comment.
CHECK_FEATURE_RETURN_EMPTY? (I can't find definitions of these macros, but it seems that the caller expects null to be returned on failure.)
There was a problem hiding this comment.
Please validate. Follow up with me if there's something in WIL or something more standard we can use instead throughout our sample app code.
| } | ||
| // Query for the ICoreWebView2_17 interface to access the Find feature. | ||
| auto webView2_17 = m_webView.try_query<ICoreWebView2_17>(); | ||
| CHECK_FEATURE_RETURN(webView2_17); |
There was a problem hiding this comment.
CHECK_FEATURE_RETURN_FALSE?
There was a problem hiding this comment.
Please validate if this is the correct macro
|
|
||
| // Specify that a custom UI will be used for the find operation. | ||
| webView.CoreWebView2.Find.SuppressDefaultDialog = true; | ||
| webView.CoreWebView2.Find.ShouldHighlightAllMatches = true; |
There was a problem hiding this comment.
It's not obvious at first glance which things are "find configuration" things and which are "find things", particularly since "ShouldHighlightAllMatches" is configuring the default Find UI!
Maybe call one of them FindRequest and the other FindUI?
There was a problem hiding this comment.
Please fix sample code. These properties are now on the Configuration / Options object.
FindOnPage.md
Outdated
| #### .NET C# | ||
| ```csharp | ||
| //! [GetActiveMatchIndex] | ||
| public async Task<int> GetActiveMatchIndexAsync() |
There was a problem hiding this comment.
This method contains no await statements. No need to be async.
|
|
||
| /// Navigates to the next match in the document. | ||
| // MSOWNERS: core ([email protected]) | ||
| HRESULT FindNext( |
There was a problem hiding this comment.
It seems that there are two ways to perform a FindNext.
- Call StartFind with the same configuration as the currently active Find operation. ("Starting another [find session] with the same configuration will adjust the active match index based on the selected Find Direction.")
- Call FindNext explicitly.
Does this make FindNext redundant?
There was a problem hiding this comment.
See above. We said StartFind will now always start a new/fresh find session.
|
In my case guys, I am hoping that with this API I can simply trigger the default ui for WebView2 to handle things going forward itself.
Sent from Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Raymond Chen ***@***.***>
Sent: Tuesday, May 21, 2024 12:56:40 AM
To: MicrosoftEdge/WebView2Feedback ***@***.***>
Cc: ajtruckle ***@***.***>; Comment ***@***.***>
Subject: Re: [MicrosoftEdge/WebView2Feedback] Create FindOnPage.md (PR #4399)
@oldnewthing commented on this pull request.
________________________________
In FindOnPage.md<#4399 (comment)>:
+The WebView2Find API offers methods and events for text finding and navigation
+within a WebView2 control. It enables developers to programmatically initiate find
+operations, navigate find results, suppress default UI, and customize find configurations
+like query and find direction. It also tracks the status of operations, indicating
+completion, match count changes, and match index changes.
+
+## Examples
+
+
+#### Description
+
+To initiate a find operation in a WebView2 control, use the `StartFind` method.
+This method allows setting the find term and find parameters via the
+`ICoreWebView2FindConfiguration` interface. Only one find session can be active per
+webview environment. Starting another with the same configuration will adjust
+the active match index based on the selected Find Direction.
What about starting another find session with a different configuration? Does that implicitly cancel the previous find session?
________________________________
In FindOnPage.md<#4399 (comment)>:
+{
+ // Query for the ICoreWebView2Environment5 interface.
+ auto webView2Environment5 = m_webViewEnvironment.try_query<ICoreWebView2Environment5>();
+ CHECK_FEATURE_RETURN(webView2Environment5);
+
+ // Initialize find configuration/settings
+ wil::com_ptr<ICoreWebView2FindConfiguration> findConfiguration;
+ CHECK_FAILURE(webView2Environment5->CreateFindConfiguration(&findConfiguration));
+ CHECK_FAILURE(findConfiguration->put_FindTerm(findTerm.c_str()));
+ CHECK_FAILURE(findConfiguration->put_IsCaseSensitive(false));
+ CHECK_FAILURE(findConfiguration->put_ShouldMatchWord(false));
+ CHECK_FAILURE(findConfiguration->put_FindDirection(COREWEBVIEW2_FIND_DIRECTION_FORWARD));
+
+ // Query for the ICoreWebView2_17 interface to access the Find feature.
+ auto webView2_17 = m_webView.try_query<ICoreWebView2_17>();
+ CHECK_FEATURE_RETURN(webView2_17);
Since this is just a feature check (we throw the QI away), shouldn't this be done first, before we ask for ICoreWebView2Environment5?
________________________________
In FindOnPage.md<#4399 (comment)>:
+To initiate a find operation in a WebView2 control, use the `StartFind` method.
+This method allows setting the find term and find parameters via the
+`ICoreWebView2FindConfiguration` interface. Only one find session can be active per
+webview environment. Starting another with the same configuration will adjust
+the active match index based on the selected Find Direction.
+### Create/Specify a Find Configuration
+#### WIN32 C++
+
+```cpp
+
+//! [InitializeFindConfiguration]
+wil::com_ptr<ICoreWebView2FindConfiguration> AppWindow::InitializeFindConfiguration(const std::wstring& findTerm)
+{
+ // Query for the ICoreWebView2Environment5 interface.
+ auto webView2Environment5 = m_webViewEnvironment.try_query<ICoreWebView2Environment5>();
+ CHECK_FEATURE_RETURN(webView2Environment5);
CHECK_FEATURE_RETURN_EMPTY? (I can't find definitions of these macros, but it seems that the caller expects null to be returned on failure.)
________________________________
In FindOnPage.md<#4399 (comment)>:
+}
+//! [InitializeFindConfiguration]
+```
+
+```cpp
+//! [ExecuteFindWithDefaultUI]
+bool AppWindow::ConfigureAndExecuteFind(const std::wstring& findTerm)
+{
+ auto findConfiguration = InitializeFindConfiguration(findTerm);
+ if (!findConfiguration)
+ {
+ return false;
+ }
+ // Query for the ICoreWebView2_17 interface to access the Find feature.
+ auto webView2_17 = m_webView.try_query<ICoreWebView2_17>();
+ CHECK_FEATURE_RETURN(webView2_17);
CHECK_FEATURE_RETURN_FALSE?
________________________________
In FindOnPage.md<#4399 (comment)>:
+ // Get the Find interface.
+ wil::com_ptr<ICoreWebView2Find> webView2Find;
+ CHECK_FAILURE(webView2_17->get_Find(&webView2Find));
+
+ // By default Find will use the default UI and highlight all matches. If you want different behavior
+ // you can change the SuppressDefaultDialog and ShouldHighlightAllMatches properties here.
+
+ // Start the find operation with a callback for completion.
+ CHECK_FAILURE(webView2Find->StartFind(
+ findConfiguration.get(),
+ Callback<ICoreWebView2FindOperationCompletedHandler>(
+ [this](HRESULT result, BOOL status) -> HRESULT
+ {
+ if (SUCCEEDED(result))
+ {
+ // Optionally update UI elements here upon successful find operation.
If no matches are found, is that a "successful" find operation?
________________________________
In FindOnPage.md<#4399 (comment)>:
+ {
+ throw new InvalidOperationException("WebView2 is not initialized.");
+ }
+
+ // Initialize the find configuration with specified settings.
+ var findConfiguration = new CoreWebView2FindConfiguration
+ {
+ FindTerm = findTerm,
+ IsCaseSensitive = false,
+ ShouldMatchWord = false,
+ FindDirection = CoreWebView2FindDirection.Forward
+ };
+
+ // Specify that a custom UI will be used for the find operation.
+ webView.CoreWebView2.Find.SuppressDefaultDialog = true;
+ webView.CoreWebView2.Find.ShouldHighlightAllMatches = true;
It's not obvious at first glance which things are "find configuration" things and which are "find things", particularly since "ShouldHighlightAllMatches" is configuring the default Find UI!
Maybe call one of them FindRequest and the other FindUI?
________________________________
In FindOnPage.md<#4399 (comment)>:
+ var findConfiguration = new CoreWebView2FindConfiguration
+ {
+ FindTerm = findTerm,
+ IsCaseSensitive = false,
+ ShouldMatchWord = false,
+ FindDirection = CoreWebView2FindDirection.Forward
+ };
+
+ // Specify that a custom UI will be used for the find operation.
+ webView.CoreWebView2.Find.SuppressDefaultDialog = true;
+ webView.CoreWebView2.Find.ShouldHighlightAllMatches = true;
+
+ // Start the find operation with the specified configuration.
+ await webView.CoreWebView2.Find.StartFindAsync(findConfiguration);
+ // It's expected that the custom UI for navigating between matches (next, previous)
+ // and stopping the find operation will be managed by the developer's custom code.
Does the custom code have the ability to push the ActiveMatchIndex back into the WebView? (So that other plugins can read it / be notified when it changes?) Or is the idea that the custom UI should just respond to the active match but not try to change it? When the user hits "Next" in the custom UI, the custom UI should perform a StartFind?
I think an example of a custom UI would help.
________________________________
In FindOnPage.md<#4399 (comment)>:
+
+ // Register ActiveMatchIndexChanged event handler
+ m_webView->add_ActiveMatchIndexChanged(
+ Callback<ICoreWebView2FindActiveMatchIndexChangedEventHandler>(
+ [this](LONG activeMatchIndex) -> HRESULT
+ {
+ // Update custom UI
+ wprintf(L"Active Match Index Changed: %ld\n", activeMatchIndex);
+ return S_OK;
+ }).Get(),
+ &m_activeMatchIndexChangedToken);
+```
+#### .NET C#
+```csharp
+//! [GetActiveMatchIndex]
+public async Task<int> GetActiveMatchIndexAsync()
This method contains no await statements. No need to be async.
________________________________
In FindOnPage.md<#4399 (comment)>:
+// MSOWNERS: core ***@***.***)
+[uuid(9c494a0a-c5d8-5fee-b7e6-4926d8d7b391), object, pointer_default(unique)]
+interface ICoreWebView2StagingFind : IUnknown {
+ /// Retrieves the index of the currently active match in the find session. Returns the index of the currently active match, or -1 if there is no active match.
+ // MSOWNERS: core ***@***.***)
+ [propget] HRESULT ActiveMatchIndex([out, retval] UINT32* value);
+
+
+ /// Gets the total count of matches found in the current document based on the last find sessions criteria. Returns the total count of matches.
+ // MSOWNERS: core ***@***.***)
+ [propget] HRESULT MatchCount([out, retval] UINT32* value);
+
+
+ /// Gets the `ShouldHighlightAllMatches` property.
+ // MSOWNERS: core ***@***.***)
+ [propget] HRESULT ShouldHighlightAllMatches([out, retval] BOOL* value);
No event for when this changes? How would the custom UI know when to update its checkbox?
Similarly, how does the custom UI know when to dismiss (when StopFind is called)? How does the custom UI know whether to show the "case sensitive" box checked or unchecked? It seems that the current API is not sufficient to build the custom Find UI.
________________________________
In FindOnPage.md<#4399 (comment)>:
+
+
+ /// Initiates a find using the specified configuration.
+ /// Displays the Find bar and starts the find operation. If a find session was already ongoing, it will be stopped and replaced with this new instance.
+ /// If called with an empty string, the Find bar is displayed but no finding occurs. Changing the configuration object after initiation won't affect the ongoing find session.
+ /// To change the ongoing find session, StartFind must be called again with a new or modified configuration object.
+ /// This method is primarily designed for HTML document queries.
+ // MSOWNERS: core ***@***.***)
+ HRESULT StartFind(
+ [in] ICoreWebView2StagingFindConfiguration* configuration
+ , [in] ICoreWebView2StagingFindStartFindCompletedHandler* handler
+ );
+
+ /// Navigates to the next match in the document.
+ // MSOWNERS: core ***@***.***)
+ HRESULT FindNext(
It seems that there are two ways to perform a FindNext.
1. Call StartFind with the same configuration as the currently active Find operation. ("Starting another [find session] with the same configuration will adjust the active match index based on the selected Find Direction.")
2. Call FindNext explicitly.
Does this make FindNext redundant?
________________________________
In FindOnPage.md<#4399 (comment)>:
+# WebView2Find API
+
+## Background
+
+The WebView2Find API offers methods and events for text finding and navigation
+within a WebView2 control. It enables developers to programmatically initiate find
+operations, navigate find results, suppress default UI, and customize find configurations
+like query and find direction. It also tracks the status of operations, indicating
+completion, match count changes, and match index changes.
+
+## Examples
+
+
+#### Description
+
+To initiate a find operation in a WebView2 control, use the `StartFind` method.
The old API (Microsoft internal link<https://microsoft.sharepoint.com/:w:/r/teams/OSG_DevX/_layouts/15/Doc.aspx?sourcedoc=%7bFB431E6D-55C4-4088-8AE6-F5923F66CFCC%7d&file=WebPlatform+Find+On+Page.docx&action=default&mobileredirect=true&DefaultItemOpen=1&share=IQFtHkP7xFWIQIrm9ZI_Zs_MAd4xulCXvVszPGX1lg5jenc>) called it a "find session", initiated by a "find request".
—
Reply to this email directly, view it on GitHub<#4399 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB45RMZDA4DUYLLVWOXRHOTZDKETRAVCNFSM6AAAAABD43NYUSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANRXGIYDINZZGA>.
You are receiving this because you commented.Message ID: ***@***.***>
|
| try | ||
| { | ||
| // Check if the webView is already initialized and is an instance of CoreWebView2. | ||
| if (webView.CoreWebView2 == null) |
There was a problem hiding this comment.
Sample: just call EnsureWebView2Async() instead.
There was a problem hiding this comment.
Please fix: Calling Ensure may be easier to understand. Please use that (assuming it doesn't otherwise cause issues for the sample).
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
I think part of the awkwardness is usage of the word "find" rather than "search"; "search" as both a noun and a verb is more common, at least in en-us. (But "find" is the right name for this feature)
FindOnPage.md
Outdated
| wil::com_ptr<ICoreWebView2FindConfiguration> findConfiguration; | ||
| CHECK_FAILURE(webView2Environment5->CreateFindConfiguration(&findConfiguration)); | ||
| CHECK_FAILURE(findConfiguration->put_FindTerm(findTerm.c_str())); | ||
| CHECK_FAILURE(findConfiguration->put_IsCaseSensitive(false)); |
There was a problem hiding this comment.
Is false not the default? We shouldn't show setting the default in examples, as it confuses what the default is
There was a problem hiding this comment.
Please fix. Ensure the ref docs say what the default value is. If the default matches what you are setting here then do not explicitly set it to the default.
FindOnPage.md
Outdated
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. | ||
| This method allows setting the find term and find parameters via the | ||
| `ICoreWebView2FindConfiguration` interface. Only one find session can be active per |
There was a problem hiding this comment.
This kind of class is usually called an Options
There was a problem hiding this comment.
Please fix
- CoreWebView2FindConfiguration -> CoreWebView2FindOptions
| //! [ConfigureAndExecuteFindWithDefaultUI] | ||
| async Task ConfigureAndExecuteFindWithDefaultUIAsync(string findTerm) | ||
| { | ||
| try |
There was a problem hiding this comment.
We shouldn't show try/catch in examples as it implies it's necessary. (And if it's necessary we should make it unnecessary.)
There was a problem hiding this comment.
Please fix. Only include try/catch in sample code if its actually required for the particular sample scenario (not just as a good idea / catch all)
| //! [GetActiveMatchIndex] | ||
| public async Task<int> GetActiveMatchIndexAsync() | ||
| { | ||
| var webViewFind = webView.CoreWebView2.Find; // Assuming webView is your WebView2 control |
There was a problem hiding this comment.
Sample nit: make it a class member and declare it
There was a problem hiding this comment.
Please fix (make it _webView)
FindOnPage.md
Outdated
| This method allows setting the find term and find parameters via the | ||
| `ICoreWebView2FindConfiguration` interface. Only one find session can be active per | ||
| webview environment. Starting another with the same configuration will adjust | ||
| the active match index based on the selected Find Direction. |
There was a problem hiding this comment.
I read this as really this always starts a Find session with the first match active. You get the same result whether or not you had the same find already running. Took me a minute to figure out, but if that's what it means, then the behavior sounds right
FindOnPage.md
Outdated
|
|
||
| ```cpp | ||
|
|
||
| //! [InitializeFindConfiguration] |
There was a problem hiding this comment.
I think it's used to extract the example code into a doc somewhere
| // you can change the SuppressDefaultDialog and ShouldHighlightAllMatches properties here. | ||
|
|
||
| // Start the find operation with a callback for completion. | ||
| CHECK_FAILURE(webView2Find->StartFind( |
There was a problem hiding this comment.
The IDL calls this StartFindAsync, but I think "Start" and "Async" are redundant, we should go with one or the other
There was a problem hiding this comment.
Please fix:
- COM: Start
- .NET/WinrT: StartAsync
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
Please update:
- CoreWebView2.Find -> CoreWebView2.FindSession
- StartFindAsync -> StartAsync
- StopFind -> Stop
(but leave FindNext/Previous as is with Find)
FindOnPage.md
Outdated
|
|
||
| #### Description | ||
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. |
There was a problem hiding this comment.
Please fix after renaming property name its now FindSession (including casing) throughout doc.
FindOnPage.md
Outdated
|
|
||
| To initiate a find operation in a WebView2 control, use the `StartFind` method. | ||
| This method allows setting the find term and find parameters via the | ||
| `ICoreWebView2FindConfiguration` interface. Only one find session can be active per |
There was a problem hiding this comment.
Please fix
- CoreWebView2FindConfiguration -> CoreWebView2FindOptions
|
|
||
|
|
||
| /// Adds an event handler for the `ActiveMatchIndexChanged` event. | ||
| /// Registers an event handler for the ActiveMatchIndexChanged event. This event is raised when the index of the currently active match changes. This can happen when the user navigates to a different match or when the active match is changed programmatically. The parameter is the event handler to be added. Returns a token representing the added event handler. This token can be used to unregister the event handler. |
| /// Displays the Find bar and starts the find operation. If a find session was already ongoing, it will be stopped and replaced with this new instance. | ||
| /// If called with an empty string, the Find bar is displayed but no finding occurs. Changing the configuration object after initiation won't affect the ongoing find session. | ||
| /// To change the ongoing find session, StartFind must be called again with a new or modified configuration object. | ||
| /// This method is primarily designed for HTML document queries. |
There was a problem hiding this comment.
Please fix.
This comment is out of date. The comment on the MIDL3 StartFindAsync is more up to date.
|
|
||
| /// Navigates to the next match in the document. | ||
| // MSOWNERS: core ([email protected]) | ||
| HRESULT FindNext( |
There was a problem hiding this comment.
Correct - Please explicitly state that in the ref docs for FindNext/Previous.
|
|
||
| /// Navigates to the next match in the document. | ||
| // MSOWNERS: core ([email protected]) | ||
| HRESULT FindNext( |
There was a problem hiding this comment.
See above. We said StartFind will now always start a new/fresh find session.
FindOnPage.md
Outdated
| [propget] HRESULT FindDirection([out, retval] COREWEBVIEW2_FIND_DIRECTION* value); | ||
|
|
||
|
|
||
| /// |
There was a problem hiding this comment.
If this is generated by our tooling, please follow up with me/Jessica about generating ref docs for these.
| ### Setting Up Find Configuration with MIDL3 | ||
|
|
||
| ### CoreWebView2 Find Configuration and Direction | ||
|
|
| /// Note: Changes to this property take effect only when StartFind, FindNext, or FindPrevious is called. | ||
| /// Preferences for the session cannot be updated unless another call to the StartFind function on the server-side is made. | ||
| /// Therefore, changes will not take effect until one of these functions is called. | ||
| Boolean SuppressDefaultFindDialog { get; set; }; |
There was a problem hiding this comment.
Please fix - the property should be on the config
FindOnPage.md
Outdated
| /// To change the ongoing find session, StartFindAsync must be called again with a new or modified configuration object. | ||
| /// StartFind supports, HTML, PDF, and TXT document queries. In general this api is designed for text based find sessions. | ||
| //// If you start a find session programmatically on another file format that doesnt have text fields, the find session will try to execute but will fail to find any matches. (It will silently fail) | ||
| /// Note: The asynchronous action completes when the UI has been displayed with the find term in the UI bar, and the matches have populated on the counter on the find bar. |
There was a problem hiding this comment.
Is the completion of the IAsyncAction the point at which calls to Next()/Previous()/Stop() methods become allowed?
Yes
What happens if I call Next() before this IAsyncAction completes, is it the same "silently fail" as when there's no active session?
Yes
Please make sure this is documented in the section about Start and what its async completion means.
(One with and One without the filtering options)
Co-authored-by: MikeHillberg <[email protected]>
Co-authored-by: MikeHillberg <[email protected]>
Co-authored-by: FrankC-msft <[email protected]>
Removed staging from interface names
replaced web view with WebView2
updated description of find next and previous to clarify that they dont update properties
removed Async from getactivematchindex
changed startfindasync to startasync
Removed Find Direction and updated documentation
This is a review for the new FindOnPage API.