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

Initial Prototype of Find in HtmlPreviewPart #22918

Closed
wants to merge 1 commit into from

Conversation

hoovercj
Copy link
Member

This is what I came up with to address #2187.

I based it on the FindWidget and in fact have imported some code from there, and for the moment have duplicated some of the CSS as well. Consider that a WIP to be cleaned up if this overall approach is accepted.

The main parts of the pull request are:

  • SimpleFindState: Maintains the visible state of the widget and the state of the results. Notifies listeners when they change
  • Webview: add hooks for finding in the page and emitting results
  • FindModelBoundToWebView: As the name implies, this is the model that is bound to the webview implementation details. It updates the state when the webview returns results
  • SimpleFindWidget: Has the input and buttons, receives state changes to update the text and buttons and asks the model to search when buttons are pressed
  • HtmlPreviewPart: Composes everything.

Thoughts?

htmlfinddemo

@mention-bot
Copy link

@hoovercj, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @jrieken to be potential reviewers.

@msftclas
Copy link

@hoovercj,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@hoovercj
Copy link
Member Author

tagging @mjbvz as well

@mjbvz mjbvz requested review from jrieken, mjbvz and alexdima March 20, 2017 23:41
@mjbvz
Copy link
Collaborator

mjbvz commented Mar 21, 2017

This looks pretty sweet. Nice work!

I can provide more feedback tomorrow.� Also, @jrieken and @alexandrudima know much more about existing find implementation and should be able to provide better feedback/advice.

The overall approach seems solid to me at least, with my main concern at present being the amount duplicated code and styling. It sounds like you are also aware of this issue and are working to address it. I could see FindWidget either becoming an abstract class that is specialized for the editor and for the webview, or, perhaps better, having the FindWidget some additional set of adapters/config-objects that specialize its behavior and appearance based on what is needed

@hoovercj
Copy link
Member Author

hoovercj commented Jul 2, 2017

Superceded by #30016

@hoovercj hoovercj closed this Jul 2, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants