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

Viewer pane pops out to editor tab #4808

Merged
merged 17 commits into from
Nov 21, 2024
Merged

Viewer pane pops out to editor tab #4808

merged 17 commits into from
Nov 21, 2024

Conversation

isabelizimm
Copy link
Contributor

@isabelizimm isabelizimm commented Sep 24, 2024

addresses #3714

Example video (bonus content: if you turn volume up you can hear puppy snoring 💤 )

editor-preview.mov

Can pop out HTML or url from viewer pane to editor tab. Should work for multiple editors, independently of the Viewer pane content.

Known problems/what is not included:

  • cannot go directly from Preview button in Quarto file to editor. It seems like Quarto has a different toolbar than urls/html (same with html sent to output, eg, No way to clear viewer with great-tables table #4601)
  • no ability to go from HTML file directly to editor
  • no toggle as an experimental feature, but happy to add that in if we think its the right choice!

very inspired by #4364 🫶

QA notes

These are all gestures I've gone through and confirmed working! It looks like a lot of steps but includes a lot of opening up files 😄

  • Have 2 distinct.qmd files
  • quarto render filename_1.qmd in Terminal so you have HTML output
  • Open HTML in Viewer
  • Click editor button (icon shown in video) to open in editor
  • Resize newly opened editor, should not overlap
  • With filename_1.qmd still in editor,quarto preview filename_2.qmd to get a localhost url
  • Click on url and open in Viewer
  • Click editor button so you have both .qmd files in editors
  • Switch between editors, should reopen prior content
  • Clear Viewer pane, should affect editors
  • Close editors

@isabelizimm isabelizimm marked this pull request as ready for review November 18, 2024 21:01
@@ -30,7 +30,7 @@ export class PreviewHtml extends PreviewWebview {
previewId: string,
webview: PreviewOverlayWebview,
readonly uri: URI,
readonly html: ShowHtmlFileEvent
readonly html?: ShowHtmlFileEvent
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was edited to allow creation of PreviewUrl independent of a ShowHtmlFileEvent

@@ -496,4 +501,35 @@ export class PositronPreviewService extends Disposable implements IPositronPrevi
// It's a localhost http or https URL; we can handle it in the viewer.
return true;
}

public async openEditor(uri: URI, title?: string): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is where most of the content is; where we create the preview webview and open it in a custom editor. afaict, we are able to use PreviewUrl for HTML files and local URLs, since they all will have a uri we can use to open.

@isabelizimm isabelizimm changed the title wip: viewer pane pops out to editor tab Viewer pane pops out to editor tab Nov 18, 2024
@isabelizimm isabelizimm requested review from nstrayer and removed request for jmcphers November 19, 2024 15:01
@timtmok
Copy link
Contributor

timtmok commented Nov 20, 2024

I see some unexpected behaviour with switching between editors. It goes away once I close the preview editor tab. Somehow, the preview contents are always on top.

Screen.Recording.2024-11-20.at.11.18.17.AM.mov

@isabelizimm
Copy link
Contributor Author

Oh gosh, you are correct 😭 should be fixed now with a proper visibility event handler!

@timtmok
Copy link
Contributor

timtmok commented Nov 20, 2024

Oh gosh, you are correct 😭 should be fixed now with a proper visibility event handler!

I'm getting a blank editor on first view now. It seems to fix itself when switching to another editor tab and back. It might be something to do with the initial visibility flag on the editor?

The preview service is using the webview service to create an overlay webview, which is sitting on top of the editor area. Maybe using a regular, non-overlay webview will make it easier to deal with visibility?

@isabelizimm
Copy link
Contributor Author

isabelizimm commented Nov 20, 2024

It might be something to do with the initial visibility flag on the editor?

Ah, this is it! super.isVisible() is returning false on the first call, since it's not yet visible. It should probably just be true, since the first time we open up an editor we'll always want to send focus/visibility there. Updated to reflect that.

Maybe using a regular, non-overlay webview will make it easier to deal with visibility?

I had set it up this way to be able to reuse the webviews from the Viewer Pane, which require an overlay webview. I think the visibility was just me making it more complicated than it needed to be 😆 BUT if there are other important reasons to not use an overlay, I can look at teasing those two pieces apart!

timtmok
timtmok previously approved these changes Nov 21, 2024
Copy link
Contributor

@timtmok timtmok left a comment

Choose a reason for hiding this comment

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

Looks good!

It will be interesting to see if there's feedback about bringing back the editor when the UI refreshes (might be useful for Workbench). The previews aren't currently saved so that's something that could be done later, if there's interest, to support this.

nstrayer
nstrayer previously approved these changes Nov 21, 2024
Copy link
Contributor

@nstrayer nstrayer left a comment

Choose a reason for hiding this comment

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

A few nits and style suggestions but looks good to me!

…ewServiceImpl.ts

Co-authored-by: Nick Strayer <nick.strayer@posit.co>
Signed-off-by: Isabel Zimmerman <54685329+isabelizimm@users.noreply.github.com>
@isabelizimm isabelizimm dismissed stale reviews from nstrayer and timtmok via 9bc795c November 21, 2024 20:18
@isabelizimm isabelizimm merged commit e4e2247 into main Nov 21, 2024
4 checks passed
@isabelizimm isabelizimm deleted the preview-editor branch November 21, 2024 21:28
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2024
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.

3 participants