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

[mini-browser] support '.xhtml' file for preview #6969

Merged
merged 1 commit into from
Feb 5, 2020
Merged

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Jan 27, 2020

What it does

Fixes #6968

The pull-request adds support to display .xhtml files as a preview using the mini-browser.

  • Introduces a minor change to the resetBackground to also accept resetting backgrounds for .xhtml files.

How to test

  1. open a workspace
  2. open a .html file (verify that preview works using the toolbar item)
    • clicking the preview should be properly displayed with a reset background
  3. open a .xhtml file (verify that the preview works using the toolbar item)
    • clicking the preview should be properly displayed with a reset background
  4. open a .htm file (verify that the preview works using the toolbar item)
    • clicking the preview should be properly displayed with a reset background

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto [email protected]

@vince-fugnitto vince-fugnitto added the mini-browser issues related to the mini-browser label Jan 27, 2020
@vince-fugnitto vince-fugnitto self-assigned this Jan 27, 2020
@akosyakov akosyakov requested a review from jankeromnes January 28, 2020 03:15
Copy link
Member

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Oops, sorry for the late review, I missed the notification.

Approved! Works as advertised, many thanks.

I also tried to find realistic edge-cases for files ending with *html where you don't want to reset the mini-browser background, but I didn't find any (only false positives I could think of are directories ending with html, or binaries called something2html, but we don't open these in mini-browsers anyway).

packages/mini-browser/src/node/mini-browser-endpoint.ts Outdated Show resolved Hide resolved
vince-fugnitto added a commit that referenced this pull request Feb 3, 2020
Fixes #6969

With this change, `.xhtml` files can now be successfully previewed
similarly to `.html` files. The method `resetBackground` has also been
updated with a minor change to also support resetting the background for
`.xhtml` files.

Signed-off-by: Vincent Fugnitto <[email protected]>
vince-fugnitto added a commit that referenced this pull request Feb 3, 2020
Fixes #6969

This change allows previewing multiple different file types
from the `HtmlHandler` endpoint.

The supported file types include:
- `.html` (previously supported)
- `.xhtml` (newly supported)
- `.htm` (newly supported)

The resetBackground method has also been updated to support the
new file types.

Signed-off-by: Vincent Fugnitto <[email protected]>
Copy link
Member

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Perfect, thanks! 💯

One small unnecessary thing below, but still approved. 🙂 Please feel free to merge whenever it's ready.

vince-fugnitto added a commit that referenced this pull request Feb 4, 2020
Fixes #6969

This change allows previewing multiple different file types
from the `HtmlHandler` endpoint.

The supported file types include:
- `.html` (previously supported)
- `.xhtml` (newly supported)
- `.htm` (newly supported)

The resetBackground method has also been updated to support the
new file types.

Signed-off-by: Vincent Fugnitto <[email protected]>
Copy link
Member

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Many thanks! Travis windows build fail looks unrelated, so Go For Launch. 🚀

(On a side-note, I think this could have been added to the Changelog. But that’s not super important, so feel free to merge as is.)

@vince-fugnitto
Copy link
Member Author

Many thanks! Travis windows build fail looks unrelated, so Go For Launch.

(On a side-note, I think this could have been added to the Changelog. But that’s not super important, so feel free to merge as is.)

Every release I usually go over all changes during a month and create entries for fixes, new features, and breaking changes if I find any :) #7014

Fixes #6969

This change allows previewing multiple different file types
from the `HtmlHandler` endpoint.

The supported file types include:
- `.html` (previously supported)
- `.xhtml` (newly supported)
- `.htm` (newly supported)

The resetBackground method has also been updated to support the
new file types.

Signed-off-by: Vincent Fugnitto <[email protected]>
@vince-fugnitto vince-fugnitto merged commit 15b2c39 into master Feb 5, 2020
@vince-fugnitto vince-fugnitto deleted the vf/GH-6968 branch February 5, 2020 17:38
akosyakov pushed a commit to akosyakov/theia that referenced this pull request Feb 24, 2020
Fixes eclipse-theia#6969

This change allows previewing multiple different file types
from the `HtmlHandler` endpoint.

The supported file types include:
- `.html` (previously supported)
- `.xhtml` (newly supported)
- `.htm` (newly supported)

The resetBackground method has also been updated to support the
new file types.

Signed-off-by: Vincent Fugnitto <[email protected]>
JesterOrNot pushed a commit to JesterOrNot/theia that referenced this pull request Mar 12, 2020
Fixes eclipse-theia#6969

This change allows previewing multiple different file types
from the `HtmlHandler` endpoint.

The supported file types include:
- `.html` (previously supported)
- `.xhtml` (newly supported)
- `.htm` (newly supported)

The resetBackground method has also been updated to support the
new file types.

Signed-off-by: Vincent Fugnitto <[email protected]>
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.

[mini-browser] '.xhtml' files should be supported by preview
2 participants