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

fix(cli): render images in local preview #5889

Merged
merged 6 commits into from
Feb 5, 2025
Merged

Conversation

Edsun457
Copy link
Contributor

@Edsun457 Edsun457 commented Feb 4, 2025

Description

Fix ticket #5756

Cause of the issue

In local environment when re-rendering markdown files, the image rendered to browser as file relative path. The browser can not parse the image unless it's the image ID matched in FileV2

For example: <img src="./table-of-contents.png" alt="Table of contents feature" />

What this PR fixed

  • Make sure the image rendered to browser is image ID: example: <img src="file:86a5f557-626e-418e-9e7a-e9f1cdc723f2" alt="Edit this page feature" />
  • update the logic of path parser to take both relative path and absolute path

Changes Made

Add logic to populate image file id so would render the right image to the browser for local env

Testing

  • Unit tests added/updated
  • Manual testing completed

@Edsun457 Edsun457 requested a review from dsinghvi as a code owner February 4, 2025 23:17
@Edsun457 Edsun457 force-pushed the edwardsun/render-local-image branch 2 times, most recently from 1e91289 to f53163f Compare February 4, 2025 23:20
Copy link

github-actions bot commented Feb 4, 2025

@dsinghvi dsinghvi changed the title Edwardsun/render local image fix(cli): render images in local preview Feb 4, 2025
const path = "/" + file.url.replace("/_local/", "");
return [AbsoluteFilePath.of(path), id];
})
);
Copy link
Member

Choose a reason for hiding this comment

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

what is happening in this transformation here?

@dsinghvi
Copy link
Member

dsinghvi commented Feb 4, 2025

Before you merge, you'll want to add a changelog to https://github.com/fern-api/fern/blob/main/packages/cli/cli/versions.yml -- that way a new version will get published to npm

Copy link
Member

@dsinghvi dsinghvi left a comment

Choose a reason for hiding this comment

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

@Edsun457 can you fix lint and add a changelog? Other than that, we are good to go!

@Edsun457 Edsun457 force-pushed the edwardsun/render-local-image branch from 7e6a617 to 76277ca Compare February 5, 2025 17:31
@Edsun457 Edsun457 merged commit 7442b9b into main Feb 5, 2025
50 of 52 checks passed
@Edsun457 Edsun457 deleted the edwardsun/render-local-image branch February 5, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants