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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ jobs:
steps:
- uses: actions/checkout@v2
- uses: jupyterlab/maintainer-tools/.github/actions/base-setup@v1
- name: Install missing dependencies
# On May 30, 2023 this wasn't included in the base try removing in future!
run: pip install typing_extensions
Comment on lines +156 to +158
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.

- uses: jupyterlab/maintainer-tools/.github/actions/check-links@v1
with:
ignore_links: 'https:\/\/mybinder\.org.*'
18 changes: 13 additions & 5 deletions src/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -17,10 +17,18 @@ export async function imageUrlSourceTransform(
await Promise.all(
images.map(async image => {
if (!image || !image.url) return;
const resolver = new AttachmentsResolver({
parent: opts.resolver ?? undefined,
model: opts.cell.model.attachments
});
if (!opts.resolver) {
console.warn(
'No resolver supplied to `imageUrlSourceTransform`, local images may not work.'
);
return;
}
const resolver = opts.cell
? new AttachmentsResolver({
parent: opts.resolver ?? undefined,
model: opts.cell?.model.attachments
})
: opts.resolver;
const path = await resolver.resolveUrl(image.url);
if (!path) return;
const url = await resolver.getDownloadUrl(path);
Expand Down
12 changes: 9 additions & 3 deletions src/mime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { ReactWidget, UseSignal } from '@jupyterlab/apputils';
import { unified } from 'unified';
import { Signal } from '@lumino/signaling';
import React from 'react';
import { imageUrlSourceTransform } from './images';

/**
* The MIME type for Markdown.
Expand Down Expand Up @@ -120,8 +121,9 @@ export class RenderedMySTMarkdown
*
* @returns A promise which resolves when rendering is complete.
*/
renderModel(model: IRenderMime.IMimeModel): Promise<void> {
const mdast = markdownParse(model.data[MIME_TYPE] as string);
async renderModel(model: IRenderMime.IMimeModel): Promise<void> {
const markdownText = model.data[MIME_TYPE] as string;
const mdast = markdownParse(markdownText, false);
agoose77 marked this conversation as resolved.
Show resolved Hide resolved
const linkTransforms = [
new WikiTransformer(),
new GithubTransformer(),
Expand All @@ -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.

article: mdast as any
};
const { frontmatter: frontmatterRaw } = getFrontmatter(mdast, {
Expand Down Expand Up @@ -159,6 +160,11 @@ export class RenderedMySTMarkdown
.use(keysPlugin)
.runSync(mdast as any, file);

// Go through all links and replace the source if they are local
await imageUrlSourceTransform(mdast as any, {
resolver: this.resolver
});

this.state = {
mdast,
references,
Expand Down
10 changes: 6 additions & 4 deletions src/myst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { internalLinksPlugin } from './links';
import { addCiteChildrenPlugin } from './citations';
import { evalRole } from './roles';

export function markdownParse(text: string): Root {
export function markdownParse(text: string, inNotebook = true): Root {
const mdast = mystParse(text, {
directives: [
cardDirective,
Expand All @@ -56,9 +56,11 @@ export function markdownParse(text: string): Root {
}
})
.runSync(mdast as any);
// Lift children out of blocks for the next step
// We are working here as one cell at a time
liftChildren(mdast, 'block');
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');
}
Comment on lines +59 to +63
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.

return mdast as Root;
}

Expand Down
5 changes: 5 additions & 0 deletions style/preflight.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
background-image: none; /* 2 */
}

.myst img {
max-width: 100%;
}

.myst figure img {
display: block;
max-width: 100%;
}
Comment on lines +30 to 37
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.