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

🌉 Update images to work locally #145

Merged
merged 3 commits into from
May 30, 2023
Merged

🌉 Update images to work locally #145

merged 3 commits into from
May 30, 2023

Conversation

rowanc1
Copy link
Member

@rowanc1 rowanc1 commented May 30, 2023

This enables local images to work in the markdown preview.

image

@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch executablebooks/jupyterlab-myst/feat/images

@rowanc1 rowanc1 added the enhancement New feature or request label May 30, 2023
Copy link
Member Author

@rowanc1 rowanc1 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 comments. I think this is good to go!

@@ -6,7 +6,7 @@ import { AttachmentsResolver } from '@jupyterlab/attachments';

type Options = {
resolver: IRenderMime.IResolver | null;
cell: MarkdownCell;
cell?: MarkdownCell;
};

export async function imageUrlSourceTransform(
Copy link
Member Author

Choose a reason for hiding this comment

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

This function was updated to remove the cell as a requirement, which has an attachments model.

src/mime.tsx Show resolved Hide resolved
@@ -131,7 +133,6 @@ export class RenderedMySTMarkdown
const file = new VFile();
const references = {
cite: { order: [], data: {} },
footnotes: {},
Copy link
Member Author

Choose a reason for hiding this comment

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

This is just an update for myst which no longer separates these from the mdast.

Comment on lines +30 to 37
.myst img {
max-width: 100%;
}

.myst figure img {
display: block;
max-width: 100%;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Minor style updates to prevent crazy wide images in markdown previews.

Comment on lines +59 to +63
if (inNotebook) {
// If in the notebook, lift children out of blocks for the next step
// We are working here as one cell at a time
liftChildren(mdast, 'block');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The structure of the markdown should stay in the blocks that the markdown gave, which is not the case for cells, which MUST lift children of the blocks so that we can target and identify them.

@rowanc1
Copy link
Member Author

rowanc1 commented May 30, 2023

The link checker looks to be failing with ModuleNotFoundError: No module named 'typing_extensions' 🤔

Comment on lines +156 to +158
- name: Install missing dependencies
# On May 30, 2023 this wasn't included in the base try removing in future!
run: pip install typing_extensions
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to fix the CI, worst case it is a no-op. But it will probably be fixed in the future with an update to maintainer-tools.

Copy link
Collaborator

@agoose77 agoose77 left a comment

Choose a reason for hiding this comment

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

LGTM! We might need to revisit this with the changes needed for JupyterLab 4, but this is a good first step!

@rowanc1 rowanc1 merged commit ed89359 into main May 30, 2023
@rowanc1 rowanc1 deleted the feat/images branch May 30, 2023 22:12
@rowanc1
Copy link
Member Author

rowanc1 commented May 30, 2023

🚀

This was referenced May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants