Skip to content

Commit 41a0500

Browse files
improvement: markdown code to be completely hidden upon 'Hide Code' (#2974)
## 📝 Summary <!-- Provide a concise summary of what this pull request is addressing. If this PR fixes any issues, list them here by number (e.g., Fixes #123). --> Fixes #2678 . Markdown code will be completely hidden to offer less distraction. This PR adds Double click / Enter as shortcuts to display markdown code, similar to Jupyter notebooks. https://github.com/user-attachments/assets/8fd832de-14cf-4835-a31a-87c6843bca50 <img width="806" alt="Screenshot 2024-12-01 at 12 47 36 AM" src="https://github.com/user-attachments/assets/42290d9f-e5b6-457d-9918-84026526a28b"> - Had to make some design compromises (widening the gap btwn left buttons when there is a Collapse toggle, rly hard to get this right with various markdown sizes). If you have another idea in mind, let me know. ## 🔍 Description of Changes <!-- Detail the specific changes made in this pull request. Explain the problem addressed and how it was resolved. If applicable, provide before and after comparisons, screenshots, or any relevant details to help reviewers understand the changes easily. --> - attach a resizeObserver for markdown cells, when the cell is too short, the icons on the right will overlap. A resize observer is used to add a css class to make the icon inline - changed some props to be optional so that scratchpad will work fine - some refactors, including `temporarilyShowCode` function change from async to sync ## 📋 Checklist - [X] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [ ] I have added tests for the changes made. - [X] I have run the code and verified that it works as expected. ## 📜 Reviewers <!-- Tag potential reviewers from the community or maintainers who might be interested in reviewing this pull request. Your PR will be reviewed more quickly if you can figure out the right person to tag with @ --> @akshayka OR @mscolnick --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 9c47539 commit 41a0500

File tree

9 files changed

+420
-73
lines changed

9 files changed

+420
-73
lines changed

CONTRIBUTING.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ dependencies; follow those instructions.)
213213
For best practices on writing end-to-end tests, check out the [Best Practices
214214
doc](https://playwright.dev/docs/best-practices).
215215

216+
For frontend tests, you want to build the frontend first with `make fe` so that Playwright works on your latest changes.
217+
216218
**Run end-to-end tests.**
217219

218220
In the root directory, run:

frontend/e2e-tests/helper.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
2-
import { type Page, expect } from "@playwright/test";
2+
import { type Locator, type Page, expect } from "@playwright/test";
33
import { HotkeyProvider, type HotkeyAction } from "../src/core/hotkeys/hotkeys";
44
import path from "node:path";
55

@@ -35,6 +35,15 @@ export async function createCellBelow(opts: {
3535
}
3636
}
3737

38+
export async function openCellActions(page: Page, element: Locator) {
39+
await element.hover();
40+
await page
41+
.getByTestId("cell-actions-button")
42+
.locator(":visible")
43+
.first()
44+
.click();
45+
}
46+
3847
export async function runCell(opts: { page: Page; cellSelector: string }) {
3948
const { page, cellSelector } = opts;
4049

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/* Copyright 2024 Marimo. All rights reserved. */
2+
import { test, expect } from "@playwright/test";
3+
import { getAppUrl, resetFile } from "../playwright.config";
4+
import { openCellActions } from "./helper";
5+
6+
const appUrl = getAppUrl("title.py");
7+
8+
test.beforeEach(async ({ page }, info) => {
9+
await page.goto(appUrl);
10+
if (info.retry) {
11+
await page.reload();
12+
}
13+
});
14+
15+
test.afterEach(async () => {
16+
// Need to reset the file because this test modifies it
17+
await resetFile("title.py");
18+
});
19+
20+
test("change the cell to a markdown cell and toggle hide code", async ({
21+
page,
22+
}) => {
23+
const title = page.getByText("Hello Marimo!", { exact: true });
24+
await expect(title).toBeVisible();
25+
26+
// Convert to Markdown
27+
await openCellActions(page, title);
28+
await page.getByText("Convert to Markdown").click();
29+
await expect(title).toBeVisible();
30+
31+
// Verify markdown content
32+
const markdown = page.getByText("import marimo as mo");
33+
await expect(markdown).toBeVisible();
34+
35+
// Hide code
36+
await openCellActions(page, title);
37+
await page.getByText("Hide code").click();
38+
await expect(title).toBeVisible();
39+
40+
// Verify code editor is hidden
41+
const cellEditor = page.getByTestId("cell-editor");
42+
await expect(cellEditor).toBeHidden();
43+
44+
// Unhide code
45+
await openCellActions(page, title);
46+
await page.getByText("Show code").click();
47+
await expect(title).toBeVisible();
48+
49+
// Verify code editor is visible
50+
await expect(cellEditor).toBeVisible();
51+
});

frontend/src/components/editor/Cell.tsx

Lines changed: 127 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ import { Toolbar, ToolbarItem } from "@/components/editor/cell/toolbar";
5555
import { cn } from "@/utils/cn";
5656
import { isErrorMime } from "@/core/mime";
5757
import { getCurrentLanguageAdapter } from "@/core/codemirror/language/commands";
58+
import { HideCodeButton } from "./code/readonly-python-code";
59+
import { useResizeObserver } from "@/hooks/useResizeObserver";
60+
import type { LanguageAdapterType } from "@/core/codemirror/language/types";
5861

5962
/**
6063
* Imperative interface of the cell.
@@ -183,6 +186,8 @@ const CellComponent = (
183186
const setAiCompletionCell = useSetAtom(aiCompletionCellAtom);
184187
const [temporarilyVisible, setTemporarilyVisible] = useState(false);
185188

189+
const [languageAdapter, setLanguageAdapter] = useState<LanguageAdapterType>();
190+
186191
const disabledOrAncestorDisabled =
187192
cellConfig.disabled || status === "disabled-transitively";
188193

@@ -311,10 +316,84 @@ const CellComponent = (
311316
[cellRef, editorView],
312317
);
313318

319+
const isCellCodeShown = !cellConfig.hide_code || temporarilyVisible;
320+
const isMarkdown = languageAdapter === "markdown";
321+
const isMarkdownCodeHidden = isMarkdown && !isCellCodeShown;
322+
323+
const cellContainerRef = useRef<HTMLDivElement>(null);
324+
const canCollapse = canCollapseOutline(outline);
325+
326+
// If the cell is too short, we need to position some icons inline to prevent overlaps.
327+
// This can only happen to markdown cells when the code is hidden completely
328+
const [isCellStatusInline, setIsCellStatusInline] = useState(false);
329+
const [isCellButtonsInline, setIsCellButtonsInline] = useState(false);
330+
331+
useResizeObserver({
332+
ref: cellContainerRef,
333+
skip: !isMarkdown,
334+
onResize: (size) => {
335+
const shouldBeInline =
336+
(size.height && size.height < 68 && hasOutput) || false;
337+
setIsCellStatusInline(shouldBeInline);
338+
339+
if (canCollapse && shouldBeInline) {
340+
setIsCellButtonsInline(shouldBeInline);
341+
} else if (isCellButtonsInline) {
342+
setIsCellButtonsInline(false);
343+
}
344+
},
345+
});
346+
347+
// DOM node where the editorView will be mounted
348+
const editorViewParentRef = useRef<HTMLDivElement>(null);
349+
350+
// if the cell is hidden, show the code editor temporarily
351+
const temporarilyShowCode = useCallback(() => {
352+
if (!isCellCodeShown) {
353+
setTemporarilyVisible(true);
354+
editorView.current?.focus();
355+
// Reach one parent up
356+
const parent = editorViewParentRef.current?.parentElement;
357+
358+
const handleFocusOut = () => {
359+
requestAnimationFrame(() => {
360+
if (!parent?.contains(document.activeElement)) {
361+
// Hide the code editor
362+
setTemporarilyVisible(false);
363+
editorView.current?.dom.blur();
364+
parent?.removeEventListener("focusout", handleFocusOut);
365+
}
366+
});
367+
};
368+
parent?.addEventListener("focusout", handleFocusOut);
369+
}
370+
}, [isCellCodeShown, editorView]);
371+
372+
const showHiddenMarkdownCode = () => {
373+
if (isMarkdownCodeHidden) {
374+
temporarilyShowCode();
375+
}
376+
};
377+
314378
const hasOutput = !isOutputEmpty(output);
315379

380+
const unhideCode = useEvent(() => {
381+
// Fire-and-forget save
382+
void saveCellConfig({ configs: { [cellId]: { hide_code: false } } });
383+
updateCellConfig({ cellId, config: { hide_code: false } });
384+
385+
// focus the editor
386+
editorView.current?.focus();
387+
return false;
388+
});
389+
390+
// When markdown cell is hidden & output is cleared, show the code editor
391+
if (isMarkdownCodeHidden && !hasOutput) {
392+
unhideCode();
393+
}
394+
316395
const outputArea = hasOutput && (
317-
<div className="relative">
396+
<div className="relative" onDoubleClick={showHiddenMarkdownCode}>
318397
<div className="absolute top-5 -left-8 z-10 print:hidden">
319398
<CollapseToggle
320399
isCollapsed={isCollapsed}
@@ -325,7 +404,7 @@ const CellComponent = (
325404
collapseCell({ cellId });
326405
}
327406
}}
328-
canCollapse={canCollapseOutline(outline)}
407+
canCollapse={canCollapse}
329408
/>
330409
</div>
331410
<OutputArea
@@ -335,6 +414,12 @@ const CellComponent = (
335414
cellId={cellId}
336415
stale={outputStale}
337416
/>
417+
{isMarkdownCodeHidden && (
418+
<HideCodeButton
419+
className="z-20 relative -top-3"
420+
onClick={temporarilyShowCode}
421+
/>
422+
)}
338423
</div>
339424
);
340425

@@ -350,8 +435,6 @@ const CellComponent = (
350435

351436
const HTMLId = HTMLCellId.create(cellId);
352437

353-
const isCellCodeShown = !cellConfig.hide_code || temporarilyVisible;
354-
355438
// Register hotkeys on the cell instead of the code editor
356439
// This is in case the code editor is hidden
357440
useHotkeysOnElement(editing ? cellRef : null, {
@@ -416,6 +499,10 @@ const CellComponent = (
416499
moveToNextCell({ cellId, before: true, noCreate: true });
417500
return true;
418501
},
502+
Enter: () => {
503+
showHiddenMarkdownCode();
504+
return false;
505+
},
419506
// only register j/k movement if the cell is hidden, so as to not
420507
// interfere with editing
421508
...(userConfig.keymap.preset === "vim" && !isCellCodeShown
@@ -462,6 +549,21 @@ const CellComponent = (
462549
setAiCompletionCell({ cellId, initialPrompt: opts.prompt });
463550
};
464551

552+
const cellStatusComponent = (
553+
<CellStatusComponent
554+
status={status}
555+
staleInputs={staleInputs}
556+
interrupted={interrupted}
557+
editing={editing}
558+
edited={edited}
559+
disabled={cellConfig.disabled ?? false}
560+
elapsedTime={runElapsedTimeMs}
561+
runStartTimestamp={runStartTimestamp}
562+
uninstantiated={uninstantiated}
563+
lastRunStartTimestamp={lastRunStartTimestamp}
564+
/>
565+
);
566+
465567
return (
466568
<CellActionsContextMenu
467569
cellId={cellId}
@@ -481,9 +583,9 @@ const CellComponent = (
481583
canMoveX={canMoveX}
482584
title={cellTitle()}
483585
>
484-
<div className={className} id={HTMLId}>
586+
<div className={className} id={HTMLId} ref={cellContainerRef}>
485587
{userConfig.display.cell_output === "above" && outputArea}
486-
<div className="tray">
588+
<div className={cn("tray", isMarkdownCodeHidden && "!border-t-0")}>
487589
<div className="absolute right-2 -top-4 z-10">
488590
<CellToolbar
489591
edited={edited}
@@ -499,7 +601,13 @@ const CellComponent = (
499601
onRun={handleRun}
500602
/>
501603
</div>
502-
<div className="absolute flex flex-col gap-[2px] justify-center h-full left-[-34px] z-20">
604+
<div
605+
className={cn(
606+
"absolute flex flex-col gap-[2px] justify-center h-full left-[-34px] z-20",
607+
isMarkdownCodeHidden && "-top-7",
608+
isMarkdownCodeHidden && isCellButtonsInline && "-left-[3.8rem]",
609+
)}
610+
>
503611
<CreateCellButton
504612
tooltipContent={renderShortcut("cell.createAbove")}
505613
appClosed={appClosed}
@@ -532,24 +640,17 @@ const CellComponent = (
532640
clearSerializedEditorState={clearSerializedEditorState}
533641
userConfig={userConfig}
534642
editorViewRef={editorView}
643+
editorViewParentRef={editorViewParentRef}
535644
hidden={!isCellCodeShown}
536-
setTemporarilyVisible={setTemporarilyVisible}
645+
temporarilyShowCode={temporarilyShowCode}
646+
languageAdapter={languageAdapter}
647+
setLanguageAdapter={setLanguageAdapter}
537648
/>
538-
<div className="shoulder-right z-20">
539-
<CellStatusComponent
540-
status={status}
541-
staleInputs={staleInputs}
542-
interrupted={interrupted}
543-
editing={editing}
544-
edited={edited}
545-
disabled={cellConfig.disabled ?? false}
546-
elapsedTime={runElapsedTimeMs}
547-
runStartTimestamp={runStartTimestamp}
548-
uninstantiated={uninstantiated}
549-
lastRunStartTimestamp={lastRunStartTimestamp}
550-
/>
649+
<div className="shoulder-right">
650+
{!isCellStatusInline && cellStatusComponent}
551651
<div className="flex gap-2 items-end">
552652
<CellDragHandle />
653+
{isCellStatusInline && cellStatusComponent}
553654
</div>
554655
</div>
555656
<div className="shoulder-bottom hover-action">
@@ -644,7 +745,11 @@ const CellToolbar = ({
644745
config={cellConfig}
645746
hasOutput={hasOutput}
646747
>
647-
<ToolbarItem variant={"green"} tooltip={null}>
748+
<ToolbarItem
749+
variant={"green"}
750+
tooltip={null}
751+
data-testid="cell-actions-button"
752+
>
648753
<MoreHorizontalIcon strokeWidth={1.5} />
649754
</ToolbarItem>
650755
</CellActionsDropdown>

0 commit comments

Comments
 (0)