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

Changed srcElement to target in mini browser #8663

Merged

Conversation

Tpuljak
Copy link
Contributor

@Tpuljak Tpuljak commented Oct 22, 2020

Signed-off-by: Toma Puljak toma.puljak@hotmail.com

What it does

The following pull-request updates the deprecated scrElement event prop with target for the mini-browser package when handling the ENTER key to reload the page and update the content to the new location. The change is suggested by mozilla documentation.

How to test

In order to test the changes, you can use the Open URL command with some minor tweaks. Since the Open URL command uses a readonly input, it will need to be updated to show so the handler can successfully process the ENTER key code.

  1. update the following to show:

  1. rebuild the mini-browser extension using: npx run build @theia/mini-browser
  2. rebuild the example application (ex: npx run build @theia/example-electron)
  3. start the application, and using the command palette (F1) trigger the Open URL command
  4. input a URL (ex: a file on your filesystem that can be previewed such as REAMDE.md)
  5. ensure that the input can be updated, and upon pressing ENTER the content is refreshed and maps to the new location:

webview-enter

Review checklist

Reminder for reviewers

Signed-off-by: Toma Puljak <toma.puljak@hotmail.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@Tpuljak thank you for your first contribution!

Before we can review your changes please be sure to complete the following:

  • Please be sure to add a proper sign-off to your commit
  • Please be sure to sign and complete the Eclipse Contributor Agreement (ECA) with the same email as your sign-off.
  • Please be sure to update your pull-request description with a how to test section which is currently omitted. This section helps reviewers properly understand and test your changes.

@vince-fugnitto vince-fugnitto added the mini-browser issues related to the mini-browser label Oct 22, 2020
@Tpuljak
Copy link
Contributor Author

Tpuljak commented Oct 22, 2020

@vince-fugnitto I've completed everything you asked for 😄

Copy link
Contributor

@DucNgn DucNgn left a comment

Choose a reason for hiding this comment

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

The PR looks good to me 👍 . I believe more information about this change could be found here

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes work well 👍
With a little bit of changes for the how to test I was able to confirm that the handleInputChange handler correctly maps the location when ENTER is triggered:

webview-enter

@vince-fugnitto vince-fugnitto merged commit 5cd4603 into eclipse-theia:master Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mini-browser issues related to the mini-browser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants