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

Integrate httpgd #620

Merged
merged 76 commits into from
May 16, 2021
Merged

Integrate httpgd #620

merged 76 commits into from
May 16, 2021

Conversation

nx10
Copy link
Contributor

@nx10 nx10 commented Apr 14, 2021

Update: To check the PR, you need to have httpgd installed and set options(vsc.use_httpgd = TRUE) in your .Rprofile.


What problem did you solve?
This PR contains a very basic integration of httpgd to display plots.
It is intended to be a WIP PR and a continuation of (#614).

I added the implementation of a httpgd server connection backend drafted by @ManuelHentschel and removed the plot styling to make it compatible with all httpgd versions (current CRAN and GitHub master).

@ManuelHentschel will now implement a custom plot viewer that integrates with and uses vscode controls.
The development is also discussed in nx10/httpgd#63.

(This Post will be updated with screenshots and setup instructions as the development continues. Currently these are identical to #614 with the exception that all httpgd versions can be used.)


Edit: The viewer now uses the new api and includes some buttons in the titlebar to control it. In particular, it's possible to toggle between the original svg and the vscode-themed svg using the "Toggle Style" button.

image
image

@renkun-ken
Copy link
Member

Another thing I notice is that when a number of plots are produced, and I click one thumbnail to navigate to different historic plots, then there's a significant delay to show the plot (nearly 1-2 seconds).

Have you changed the settings r.httpgd.timeouts.refreshTimeout and/or r.httpgd.timeouts.resizeTimeout at some point? These can be used to control how eagerly the viewer redraws the plots and might help here. If changing these to a different value (e.g. both 100) doesn't help, please post the code that produces the laggy plots.

Cannot reproduce the delay on macOS. It is probably caused by the plot is a quite comprehensive with too many elements.

@renkun-ken
Copy link
Member

renkun-ken commented May 16, 2021

With the latest commit, the resizing seems a bit problematic that if you repeat running the same plot (e.g. plot(rnorm(100))), then the main plot area is getting smaller and smaller.

@ManuelHentschel
Copy link
Member

With the latest commit, the resizing seems a bit problematic that if you repeat running the same plot (e.g. plot(rnorm(100))), then the main plot area is getting smaller and smaller.

Thanks for noting!

package-lock.json Outdated Show resolved Hide resolved
Copy link
Member

@renkun-ken renkun-ken left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the great work!

@renkun-ken
Copy link
Member

@ManuelHentschel Would you like to update README.md (https://github.com/Ikuyadeu/vscode-R#available-functions-and-options) so that user knows about the vsc.use_httpgd option?

@ManuelHentschel ManuelHentschel merged commit 47265aa into REditorSupport:master May 16, 2021
@ManuelHentschel
Copy link
Member

Thanks for the thorough testing and constructive feedback @renkun-ken!

@nx10
Copy link
Contributor Author

nx10 commented May 16, 2021

Great work @ManuelHentschel!

@psobolewskiPhD
Copy link

The new feature works really well!
Kudos to all involved!
image

I have a couple questions:

  1. Is it possible to have the vscode theme just do color and not override my font choice (here Fira Sans).
  2. Is it possible to show/hide the history/thumbnails?
  3. the ... menu allows for export, but just svg. Is there a way to to have PNG like the original httpgd functionality?

@ManuelHentschel
Copy link
Member

Thanks for the feedback!

A quick way to achieve 1. and 2. is to e.g. comment out the line https://github.com/Ikuyadeu/vscode-R/blob/master/html/httpgd/styleOverwrites.css#L9 and change https://github.com/Ikuyadeu/vscode-R/blob/master/html/httpgd/style.css#L53 to display: none;. You can try this out by opening the installation directory of the extension (F1 > Extensions: Open Extensions Folder) and modifying the .css files directly.

I'll also add proper commands/options for this.

Regarding 3., in nx10/httpgd#69 a modular renderering approach is being implemented that will allow exporting to a number of different formats. When this is merged in httpgd, we'll also add corresponding options in vscode-R.

@psobolewskiPhD
Copy link

psobolewskiPhD commented May 22, 2021

Thank you for the clear instructions!
Two more questions:
I'm not in love with the white outline text vs. just white.
image
I thought I'd comment out the stroke and add fill: white; but that didn't work.
Is that an easy fix?
Edit: Adding !important worked! Much better!
image
Edit2: If white is too bright for folks, could use an off shade, but the outlining—to me—ruins the letter spacing and is not good.

And more importantly—to me anyways—I hate the boxes around the legend. How can I turn that off?!?

@psobolewskiPhD
Copy link

psobolewskiPhD commented May 22, 2021

I played around a bit more and got—to my eye—very nice results:
image

Editted styleOverwrites.css as follows:
image

Edit:
Better yet, I got rid of the outlines around the points!
image
I know, I'm drunk on power, deleting things!
But here's the new styleOverwrites.css
image

@nx10
Copy link
Contributor Author

nx10 commented May 22, 2021

The dynamic styling is basically just a proof of concept at this point. I plan on adding a second SVG renderer to httpgd that allows for mich easier styling at the expense of slightly increased file sizes (nx10/httpgd#65). This will be included in the next update with the modular rendering.

(It might take a couple weeks/months as I am currently busy writing my masters thesis.)

@psobolewskiPhD
Copy link

Honestly, it works great and—with the tweaks I just made—for me it looks great!
I really like the ability to easily flip plots from light vs dark mode for display purposes—saving the plots uses the normal (dark on white) colors, which is perfect.
Wonderful feature.
Kudos to you @nx10 and @ManuelHentschel plus everyone else involved in this extension.

@nx10
Copy link
Contributor Author

nx10 commented May 22, 2021

Thanks! @ManuelHentschel did most of the work on the vscode-r integration and the theming is completely his idea.

renkun-ken pushed a commit that referenced this pull request Jun 11, 2021
* Liveshare Functionality

Implements session watcher features for LiveShare guests

* Update package-lock.json

* Review changes (1)

- isLiveShare now relies on liveSession instead of a new vsls.getApi() call
-- Many functions are now synchronous as a result
- Various tooltip changes to clarify 'forward guest commands'
- Clean up helper bool functions

* Move files to liveshare folder

As per #641, liveshare files are moved to a /liveshare/ folder, and the "r" prefix has been removed from their name

* Review changes (2)

- Liveshare API handling
-- Abort on timeout
-- Retry button
- Guest & host workspace always in sync
- Guests no longer create files, instead relying on webview + virtual docs
- Fix bug with webviews returning object instead of string

* Resolve upstream changes (#2)

* Share httpgd session with guest

- Httpgd sessions are shared with guests through vsls shareBrowser functionality
- Resolve some conflicts from upstream merge

* Fix merge error + refresh tree

- Remove duplicate function declaration
- Refresh liveshare tree on retry api

* Review changes (3)

- Use liveShare.defaults as per #620
- Change overuse of brackets

* Better integrate httpgd

- The shared httpgd is now opened/reopened even if closed
- Shared httpgd is now shared even if started before liveshare
- Minor bug fixes

* *Better* integrate httpgd

- Instead of doing a workaround browser solution,  use the native httpgd webview

* Minor changes

- Change timeout default: 3000 => 10000
- Remove leftover information message
- Fix accidental merge overwrite

* Fix guest httpgd eagerness

- notifyGuestPlotManager is now called less often, preventing it from taking focus unexpectedly

* Minor changes

- .UUID -> uuid
- Shorten commandNode label

* Review changes (4)

- request() to liveShareRequest()
- onRequest() to liveShareOnRequest()
- use index design pattern
- remove global shareurl
- remove badge from README

* Lint + misc change

- Appease the linter gods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants