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

feat: resolve image paths in @penrose/editor #1018

Merged
merged 9 commits into from
May 26, 2022
Merged

feat: resolve image paths in @penrose/editor #1018

merged 9 commits into from
May 26, 2022

Conversation

wodeni
Copy link
Member

@wodeni wodeni commented May 25, 2022

Description

Related issue/PR: #1000

This PR adds support for Style-included images for (1) roger local development mode and (2) the example registry. We assume that image paths in Style are relative to the Style file parent directory, and they are all relative paths, not absolute ones.

First, WorkspaceLocation now keeps necessary data for file path resolution. Then for (1), we send a retrieve_file_from_style packet via the websocket connection (now a part of RogerState managed by Recoil), and resolve the image on file_change response with the same token. For (2), we keep track of the directory that the Style program is in and construct the URL for the image by calling new URL.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated changes to the diagrams/ folder

Open questions

  • roger will not perform HTTP requests and only does local file fetching
  • Similarly, (2) doesn't resolve absolute URLs
  • All image request are not memorized, which could cause performance problems

@codecov
Copy link

codecov bot commented May 25, 2022

Codecov Report

Merging #1018 (53bc6a7) into main (a5d5da1) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
+ Coverage   63.48%   63.49%   +0.01%     
==========================================
  Files          62       62              
  Lines        7936     7939       +3     
  Branches     1803     1802       -1     
==========================================
+ Hits         5038     5041       +3     
  Misses       2782     2782              
  Partials      116      116              
Impacted Files Coverage Δ
packages/core/src/renderer/Renderer.ts 35.44% <100.00%> (+2.54%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d5da1...53bc6a7. Read the comment docs.

@cloudflare-pages
Copy link

cloudflare-pages bot commented May 25, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 53bc6a7
Status: ✅  Deploy successful!
Preview URL: https://a46cc76a.penrose-72l.pages.dev

View logs

@wodeni wodeni self-assigned this May 25, 2022
@wodeni wodeni merged commit 7bb69e6 into main May 26, 2022
@wodeni wodeni deleted the ide-resolve-img branch May 26, 2022 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant