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

Render initial preview for empty documents #8729

Merged
merged 1 commit into from
Nov 13, 2020

Conversation

sdirix
Copy link
Member

@sdirix sdirix commented Nov 6, 2020

Improves preview-widget to also render previews for empty documents.

Fixes #8728

Signed-off-by: Stefan Dirix sdirix@eclipsesource.com
Contributed on behalf of STMicroelectronics

What it does

The preview-widget checks whether the content of the underlying text document changed before rerendering its content. In case of empty documents this check prevented the initial rendering of the preview. This is now fixed by distinguishing between empty and non-existing content.

How to test

  1. Implement a new PreviewHandler which renders content for empty documents
  2. Open preview on empty document
  3. Check whether the preview shows the content returned above

You can use this PreviewHandler for testing:

@injectable()
class MyPreviewHandler implements PreviewHandler {
  canHandle(uri: URI) {
    return 1000;
  }
  renderContent() {
    const div = document.createElement("div");
    div.innerText = "my preview";
    return div;
  }
}

Preview_Content_Shown

Review checklist

Reminder for reviewers

The preview-widget checks whether the content of the underlying text
document changed before rerendering its content. In case of empty
documents this check prevented the initial rendering of the preview.
This is now fixed by distinguishing between empty and non-existing
content.

Signed-off-by: Stefan Dirix <sdirix@eclipsesource.com>
Contributed on behalf of STMicroelectronics
@vince-fugnitto vince-fugnitto added the preview issues related to the preview label Nov 10, 2020
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 for me with the provided example.
When documents are empty, the preview is now correctly rendered.

@vince-fugnitto
Copy link
Member

I'll merge tomorrow if there are no objections.

@sdirix
Copy link
Member Author

sdirix commented Nov 12, 2020

@vince-fugnitto Thanks for the review!

@vince-fugnitto vince-fugnitto merged commit 476c8df into eclipse-theia:master Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview issues related to the preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview Widget doesn't render preview for empty documents
2 participants