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

Create FindOnPage.md #4399

Open
wants to merge 79 commits into
base: main
Choose a base branch
from
Open

Create FindOnPage.md #4399

wants to merge 79 commits into from

Conversation

maxwellmyers
Copy link

This is a review for the new FindOnPage API.

@maxwellmyers maxwellmyers added the API Proposal Review WebView2 API Proposal for review. label Feb 27, 2024
@maxwellmyers maxwellmyers marked this pull request as ready for review February 27, 2024 21:24
FindOnPage.md Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
updated midl3 definition
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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().

Copy link

Choose a reason for hiding this comment

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

I also find the naming awkward here. I think we should consider something like FindOperation or FindSession over Find.

Copy link
Contributor

Choose a reason for hiding this comment

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

The old API (Microsoft internal link) called it a "find session", initiated by a "find request".

Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should "Find" be consistently capitalized in this document when it's a noun?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix after renaming property name its now FindSession (including casing) throughout doc.

FindOnPage.md Show resolved Hide resolved
FindOnPage.md Outdated

```cpp

//! [InitializeFindConfiguration]
Copy link

Choose a reason for hiding this comment

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

Each example has a comment with this attribute-like syntax. Is this intended to signify something?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's used to extract the example code into a doc somewhere

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove from doc.

### Setting Up Find Configuration with MIDL3

### CoreWebView2 Find Configuration and Direction

Copy link

Choose a reason for hiding this comment

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

Are these sections missing? Or can these headings be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove.

FindOnPage.md Outdated
CoreWebView2FindConfiguration CreateFindConfiguration();
};

runtimeclass CoreWebView2FindConfiguration : [default]ICoreWebView2FindConfiguration {}
Copy link

Choose a reason for hiding this comment

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

fix indent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix.

FindOnPage.md Outdated
CoreWebView2FindConfiguration CreateFindConfiguration();
};

runtimeclass CoreWebView2FindConfiguration : [default]ICoreWebView2FindConfiguration {}
Copy link

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

Same comment with regards to CoreWebView2Find/ICoreWebView2Find.

Copy link
Contributor

Choose a reason for hiding this comment

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

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)
Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link

Choose a reason for hiding this comment

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

Add line breaks to keep the width of the lines at a reasonable length, e.g. 80 chars or 120 chars.

Copy link

Choose a reason for hiding this comment

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

Similarly with the rest of the long lines in this section.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix.

/// 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.
Copy link

Choose a reason for hiding this comment

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

This method is primarily designed for HTML document queries.

I don't understand what this comment means.

Copy link
Contributor

Choose a reason for hiding this comment

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

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(
Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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));
Copy link

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

See above to add screenshot and more details on the scenario.

FindOnPage.md Show resolved Hide resolved
FindOnPage.md Outdated
```cpp

/// Specifies the direction of find Parameters.
// MSOWNERS: core ([email protected])
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't normally include the MSOWNERS comments in the public doc

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix (throughout)

FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated
[propget] HRESULT FindDirection([out, retval] COREWEBVIEW2_FIND_DIRECTION* value);


///
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing descriptions for setters

Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

What about starting another find session with a different configuration? Does that implicitly cancel the previous find session?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is just a feature check (we throw the QI away), shouldn't this be done first, before we ask for ICoreWebView2Environment5?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix by moving to top of function.

{
// Query for the ICoreWebView2Environment5 interface.
auto webView2Environment5 = m_webViewEnvironment.try_query<ICoreWebView2Environment5>();
CHECK_FEATURE_RETURN(webView2Environment5);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.)

Copy link
Contributor

Choose a reason for hiding this comment

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

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

CHECK_FEATURE_RETURN_FALSE?

Copy link
Contributor

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix sample code. These properties are now on the Configuration / Options object.

FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated
#### .NET C#
```csharp
//! [GetActiveMatchIndex]
public async Task<int> GetActiveMatchIndexAsync()
Copy link
Contributor

Choose a reason for hiding this comment

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

This method contains no await statements. No need to be async.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix.

FindOnPage.md Show resolved Hide resolved

/// Navigates to the next match in the document.
// MSOWNERS: core ([email protected])
HRESULT FindNext(
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

See above. We said StartFind will now always start a new/fresh find session.

@ajtruckle
Copy link

ajtruckle commented May 21, 2024 via email

try
{
// Check if the webView is already initialized and is an instance of CoreWebView2.
if (webView.CoreWebView2 == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sample: just call EnsureWebView2Async() instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is false not the default? We shouldn't show setting the default in examples, as it confuses what the default is

Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of class is usually called an Options

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix

  • CoreWebView2FindConfiguration -> CoreWebView2FindOptions

//! [ConfigureAndExecuteFindWithDefaultUI]
async Task ConfigureAndExecuteFindWithDefaultUIAsync(string findTerm)
{
try
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't show try/catch in examples as it implies it's necessary. (And if it's necessary we should make it unnecessary.)

Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Sample nit: make it a class member and declare it

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix (make it _webView)

FindOnPage.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's used to extract the example code into a doc somewhere

FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Show resolved Hide resolved
// you can change the SuppressDefaultDialog and ShouldHighlightAllMatches properties here.

// Start the find operation with a callback for completion.
CHECK_FAILURE(webView2Find->StartFind(
Copy link
Contributor

Choose a reason for hiding this comment

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

The IDL calls this StartFindAsync, but I think "Start" and "Async" are redundant, we should go with one or the other

Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix:

  • COM: Start
  • .NET/WinrT: StartAsync

FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated

#### Description

To initiate a find operation in a WebView2 control, use the `StartFind` method.
Copy link
Contributor

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix after renaming property name its now FindSession (including casing) throughout doc.

FindOnPage.md Outdated Show resolved Hide resolved
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
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix

  • CoreWebView2FindConfiguration -> CoreWebView2FindOptions

FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Outdated Show resolved Hide resolved
FindOnPage.md Show resolved Hide resolved


/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix.

FindOnPage.md Outdated Show resolved Hide resolved
/// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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(
Copy link
Contributor

Choose a reason for hiding this comment

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

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(
Copy link
Contributor

Choose a reason for hiding this comment

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

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);


///
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove.

FindOnPage.md Outdated Show resolved Hide resolved
/// 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; };
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix - the property should be on the config

FindOnPage.md Show resolved Hide resolved
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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 Show resolved Hide resolved
maxwellmyers and others added 15 commits July 24, 2024 13:44
 (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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Proposal Review WebView2 API Proposal for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants