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

🕹 Move controls to embed node #142

Merged
merged 2 commits into from
Jul 4, 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
2 changes: 1 addition & 1 deletion packages/jupyter/src/controls/NotebookToolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export function NotebookToolbar({ showLaunch = false }: { showLaunch?: boolean }
if (computable)
return (
<div className="sticky top-[60px] flex justify-end w-full z-20 pointer-events-none">
<div className="flex p-1 m-1 space-x-1 border rounded-full shadow pointer-events-auto border-stone-300 bg-white/80 dark:bg-stone-900/80 backdrop-blur">
<div className="flex p-1 m-1 border rounded-full shadow pointer-events-auto space-x-1 border-stone-300 bg-white/80 dark:bg-stone-900/80 backdrop-blur">
{!ready && (
<div className="rounded">
<button
Expand Down
66 changes: 66 additions & 0 deletions packages/jupyter/src/embed.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { SourceFileKind, type GenericNode } from 'myst-common';
import { useCellExecution } from './execute';
import {
ArticleResetNotebook,
ArticleRunNotebook,
ArticleStatusBadge,
} from './controls/ArticleCellControls';
import { JupyterIcon } from '@scienceicons/react/24/solid';
import { select } from 'unist-util-select';
import { useLinkProvider } from '@myst-theme/providers';

function EmbedWithControls({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have moved the embed -> output node with controls here. This allows us to link to the notebook source.

I am not totally happy with it, but I think it is better than the controls being fully on the right.

outputKey,
children,
title = 'Jupyter Notebook',
url,
}: {
outputKey: string;
children?: React.ReactNode;
title?: string;
url?: string;
}) {
const { kind } = useCellExecution(outputKey);
const Link = useLinkProvider();
const showControls = kind === SourceFileKind.Article;

return (
<div className="shadow">
{showControls && (
<div className="sticky top-[60px] z-[2] w-full bg-gray-100/80 backdrop-blur dark:bg-neutral-800/80 py-1 px-2">
<div className="flex items-center">
<div className="flex items-center">
<JupyterIcon className="inline-block w-5 h-5" />
<span className="ml-2">Source:</span>
{url && (
<Link to={url} className="ml-2 no-underline text-normal hover:underline">
{title}
</Link>
)}
</div>
<div className="flex-grow" />
<ArticleStatusBadge id={outputKey} />
<ArticleRunNotebook id={outputKey} />
<ArticleResetNotebook id={outputKey} />
</div>
</div>
)}
<div className="mt-2">{children}</div>
</div>
);
}

export function Embed(node: GenericNode, children: React.ReactNode) {
const output = select('output', node) as GenericNode;
if (!output) return <>{children}</>;
return (
<EmbedWithControls
key={node.key}
outputKey={output.key}
title={node.source?.title}
url={node.source?.url}
>
{children}
</EmbedWithControls>
);
}
2 changes: 2 additions & 0 deletions packages/jupyter/src/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Embed } from './embed';
import { Output } from './output';

const OUTPUT_RENDERERS = {
output: Output,
embed: Embed,
};

export * from './BinderBadge';
Expand Down
20 changes: 2 additions & 18 deletions packages/jupyter/src/output.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import { SourceFileKind, type GenericNode } from 'myst-common';
import type { GenericNode } from 'myst-common';
import { KnownCellOutputMimeTypes } from 'nbtx';
import type { MinifiedMimeOutput, MinifiedOutput } from 'nbtx';
import classNames from 'classnames';
import { SafeOutputs } from './safe';
import { JupyterOutputs } from './jupyter';
import { useMemo } from 'react';
import { useCellExecution } from './execute';
import {
ArticleResetNotebook,
ArticleRunNotebook,
ArticleStatusBadge,
} from './controls/ArticleCellControls';

export const DIRECT_OUTPUT_TYPES = new Set(['stream', 'error']);

Expand Down Expand Up @@ -52,7 +47,7 @@ function JupyterOutput({
nodeType?: string;
align?: 'left' | 'center' | 'right';
}) {
const { kind, ready } = useCellExecution(nodeKey);
const { ready } = useCellExecution(nodeKey);
const outputs: MinifiedOutput[] = data;
const allSafe = useMemo(
() => allOutputsAreSafe(outputs, DIRECT_OUTPUT_TYPES, DIRECT_MIME_TYPES),
Expand All @@ -66,8 +61,6 @@ function JupyterOutput({
component = <JupyterOutputs id={nodeKey} outputs={outputs} />;
}

const showControls = kind === SourceFileKind.Article;

return (
<figure
id={identifier || undefined}
Expand All @@ -79,15 +72,6 @@ function JupyterOutput({
'text-right': align === 'right',
})}
>
{showControls && (
<div className="sticky top-[60px]">
<div className="absolute -top-[28px] md:top-[30px] right-0 md:-right-[28px]">
<ArticleStatusBadge id={nodeKey} />
<ArticleRunNotebook id={nodeKey} />
<ArticleResetNotebook id={nodeKey} />
</div>
</div>
)}
{component}
</figure>
);
Expand Down
2 changes: 1 addition & 1 deletion packages/site/src/components/Navigation/Navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function Navigation({
{children}
{open && (
<div
className="fixed inset-0 bg-black opacity-50"
className="fixed inset-0 z-30 bg-black opacity-50"
style={{ marginTop: top }}
onClick={() => setOpen(false)}
></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ export const TableOfContents = ({
className={classNames(
'fixed xl:article-grid article-grid-gap xl:w-screen xl:pointer-events-none overflow-auto max-xl:min-w-[300px]',
{ hidden: !open },
{ 'z-30': open },
)}
style={{
top: top ?? 0,
Expand Down
4 changes: 2 additions & 2 deletions packages/site/src/components/Navigation/TopNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function NavItem({ item }: { item: SiteNavItem }) {
leaveFrom="transform opacity-100 scale-100"
leaveTo="transform opacity-0 scale-95"
>
<Menu.Items className="absolute w-48 py-1 mt-2 origin-top-left bg-white rounded-sm shadow-lg left-4 ring-1 ring-black ring-opacity-5 focus:outline-none">
<Menu.Items className="absolute w-48 py-1 mt-2 bg-white rounded-sm shadow-lg origin-top-left left-4 ring-1 ring-black ring-opacity-5 focus:outline-none">
{item.children?.map((action) => (
<Menu.Item key={action.url}>
{/* This is really ugly, BUT, the action needs to be defined HERE or the click away doesn't work for some reason */}
Expand Down Expand Up @@ -160,7 +160,7 @@ function ActionMenu({ actions }: { actions?: SiteManifest['actions'] }) {
leaveFrom="transform opacity-100 scale-100"
leaveTo="transform opacity-0 scale-95"
>
<Menu.Items className="absolute right-0 w-48 py-1 mt-2 origin-top-right bg-white rounded-sm shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none">
<Menu.Items className="absolute right-0 w-48 py-1 mt-2 bg-white rounded-sm shadow-lg origin-top-right ring-1 ring-black ring-opacity-5 focus:outline-none">
{actions?.map((action) => (
<Menu.Item key={action.url}>
{({ active }) => (
Expand Down
2 changes: 1 addition & 1 deletion packages/site/src/pages/Article.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const ArticlePage = React.memo(function ({ article }: { article: PageLoad
</div>
)}
{canCompute && article.kind === SourceFileKind.Notebook && <NotebookToolbar showLaunch />}
{canCompute && article.kind === SourceFileKind.Article && <NotebookToolbar />}
{/* {canCompute && article.kind === SourceFileKind.Article && <NotebookToolbar />} */}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have commented this out, but we may want to bring something like it back.

Copy link
Contributor

Choose a reason for hiding this comment

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

i like the idea of making it figure oriented -- all we are losing here is the ability to "run all" in one go

<ContentBlocks pageKind={article.kind} mdast={article.mdast as GenericParent} />
<Bibliography />
<ConnectionStatusTray />
Expand Down
2 changes: 1 addition & 1 deletion packages/site/src/pages/Root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export function Document({
analytics_plausible={config?.analytics_plausible}
/>
</head>
<body className="m-0 transition-colors duration-500 bg-white dark:bg-stone-900">
<body className="m-0 bg-white transition-colors duration-500 dark:bg-stone-900">
<ThemeProvider theme={theme} renderers={renderers} {...links}>
<BaseUrlProvider baseurl={baseurl}>
<ThebeBundleLoaderProvider loadThebeLite publicPath={baseurl}>
Expand Down
4 changes: 2 additions & 2 deletions styles/block-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

@layer base {
.shaded {
@apply pt-5 bg-slate-100 dark:bg-slate-800 my-5;
@apply pt-5 my-5 bg-slate-100 dark:bg-slate-800;
}
.framed {
@apply p-5 border shadow bg-slate-50 dark:bg-slate-800 my-5;
@apply p-5 my-5 border shadow bg-slate-50 dark:bg-slate-800;
}
.shaded-children > * {
@apply p-2 bg-slate-50 dark:bg-slate-800;
Expand Down
5 changes: 3 additions & 2 deletions styles/hover.css
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.hover-card-content {
animation-duration: 0.6s;
animation-timing-function: cubic-bezier(0.16, 1, 0.3, 1);
z-index: 10;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the only real style change for z-index.

}
.hover-card-content[data-side='top'] {
animation-name: slideUp;
Expand Down Expand Up @@ -32,9 +33,9 @@
}

.hover-document {
@apply bg-white dark:bg-slate-800 text-sm rounded border border-gray-50 shadow-xl article;
@apply text-sm bg-white border rounded shadow-xl dark:bg-slate-800 border-gray-50 article;
}

.hover-link {
@apply text-blue-700 dark:text-blue-100 no-underline hover:text-blue-500 font-normal;
@apply font-normal text-blue-700 no-underline dark:text-blue-100 hover:text-blue-500;
}
4 changes: 2 additions & 2 deletions styles/typography.css
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

@layer base {
.prose table td {
@apply p-1 sm:p-2 align-top;
@apply p-1 align-top sm:p-2;
}
.prose table p,
.prose table li {
Expand Down Expand Up @@ -43,6 +43,6 @@
min-height: calc(100vh);
}
.article {
@apply prose max-w-none prose-stone dark:prose-invert break-words;
@apply prose break-words max-w-none prose-stone dark:prose-invert;
}
}