Use MarkdownEditor in dashboard #12462

kazcw · 2025-03-11T09:21:50Z

Pull Request Description

In the dashboard, replace marked with an instance of the MarkdownEditor used by project-view.

Gravacao.do.ecra.2025-03-14.as.16.42.41.mov

Fixes #12063.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
Unit tests have been written where possible.
If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
or the Snowflake database integration, a run of the Extra Tests has been scheduled.
- If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

github-actions · 2025-03-11T09:28:05Z

🧪 Storybook is successfully deployed!

📊 Dashboard:

👀 Review changes: https://www.chromatic.com/build?appId=673738989fc3e8430364197e&number=1948
👨‍🎨 Preview storybook: https://673738989fc3e8430364197e-kcqkbzhmqj.chromatic.com/

farmaazon · 2025-03-11T09:29:54Z

app/gui/src/project-view/util/codemirror/index.ts

-}) {
-  const keyboard = injectKeyboard()
+function useBindings(view: EditorView) {
+  const keyboard = injectKeyboard(true)


Perhaps we could move provideKeyboard from GraphEditor.vue to App.vue, then it should be visible in React.

farmaazon · 2025-03-11T09:38:32Z

app/gui/src/dashboard/components/MarkdownViewer/MarkdownViewer.tsx

+  const transformImageUrl: UrlTransformer = (path: string) =>
+    /^https?:/.test(path) ?
+      Promise.resolve({ ok: true, value: { url: path } })
+    : imgUrlResolver(path).then((url) => ({ ok: true, value: { url } }))


I think we should keep catching exceptions on imgUrlResolver, as in Dashboard's codebase exceptions are thrown more freely.

I think in this situation it'd be better to replace the image with an error and display the rest of the docs. So I'm voting for catching errors here or down the line.
Also, as we rely on Suspense features, we catch these errors in an 'ErrorBoundary' placed higher in the tree. But I'm not sure we can catch errors occuring in vue and outside of render/useSuspenseQuery phases

farmaazon · 2025-03-11T09:40:33Z

app/gui/src/dashboard/utilities/vue.ts

+/** Extracts the properties defined by a component, excluding various Vue internals. */
+type VueComponentProps<T> = Omit<ComponentProps<T>, keyof AllowedComponentProps | keyof VNodeProps>
+
+/** Creates a React component wrapping a Vue component. */


Jusdging by Docs, it's exactly the same as applyPureVueInReact.

Suggested change

/** Creates a React component wrapping a Vue component. */

/** Creates a Lazy React component wrapping a Vue component. */

MrFlashAccount

Did a shallow review, will do a deeper look later on

MrFlashAccount · 2025-03-11T17:27:19Z

app/gui/src/dashboard/utilities/vue.ts

+export function vueComponent<T extends { default?: unknown }>(
+  lazyImport: () => Promise<T>,
+): React.LazyExoticComponent<(props: VueComponentProps<T['default']>) => React.JSX.Element> {
+  return React.lazy(async () => {


Lazy shouldn't be mandatory, bacause it's a perf optimization and usage depends on the context.

app/gui/src/dashboard/components/MarkdownViewer/defaultRenderer.ts

MrFlashAccount · 2025-03-11T17:28:59Z

app/gui/src/dashboard/components/MarkdownViewer/MarkdownViewer.tsx

-      dangerouslySetInnerHTML={{ __html: markdownToHtml }}
-    />
-  )
+  return <MarkdownEditor content={text} transformImageUrl={transformImageUrl} toolbar={false} />


The asset docs panel has a testid; the editor can be located relative to it.

This is not sustainable, we'd like to be able to set a test id for this element

MrFlashAccount · 2025-03-11T17:34:28Z

app/gui/src/dashboard/components/MarkdownViewer/MarkdownViewer.tsx

+  const transformImageUrl: UrlTransformer = (path: string) =>
+    /^https?:/.test(path) ?
+      Promise.resolve({ ok: true, value: { url: path } })
+    : imgUrlResolver(path).then((url) => ({ ok: true, value: { url } }))


I think in this situation it'd be better to replace the image with an error and display the rest of the docs. So I'm voting for catching errors here or down the line.
Also, as we rely on Suspense features, we catch these errors in an 'ErrorBoundary' placed higher in the tree. But I'm not sure we can catch errors occuring in vue and outside of render/useSuspenseQuery phases

MrFlashAccount · 2025-03-19T19:16:34Z

app/gui/integration-test/dashboard/assetPanel.spec.ts

@@ -19,6 +19,11 @@ function locateAssetPanelDescription(page: Page) {
  return locateAssetPanel(page).getByTestId('asset-panel-description')
 }

+/** Find the contents of the Markdown editor within the given {@link Locator}. */
+function locateMarkdownContent(locator: Locator) {
+  return locator.locator('.cm-content')


className selectors are forbidden as selectors in tests

Why it is so? We are not shy of using class selector in our tests.

Because class selectors aren't sustainable. Causing false-positives because of className change isn't a good practice overall

Quick searching, here are an example of best practices from cypress website: https://docs.cypress.io/app/core-concepts/best-practices#How-It-Works

This class is part of the public API of CodeMirror. If they changed it, it would be a breaking change for a lot more than our tests.

MrFlashAccount · 2025-03-19T19:18:15Z

app/gui/src/dashboard/components/MarkdownViewer/defaultRenderer.ts

-    return `<blockquote class="${'relative my-1 pl-2 before:bg-primary/20 before:absolute before:left-0 before:top-0 before:h-full before:w-[1.5px] before:rounded-full'}">${this.parser.parse(tokens)}</blockquote>`
-  },
-}
+export const MarkdownEditor = lazyVueComponent(() => import('@/components/MarkdownEditor.vue'))


Lazy should be declared in place, not globally.

MrFlashAccount

LGTM

In the dashboard, replace `marked` with an instance of the `MarkdownEditor` used by `project-view`. https://github.com/user-attachments/assets/6b627a57-70ec-4b93-8967-35d0929c79d8 Fixes #12063.

Use MarkdownEditor in dashboard

096937c

kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 11, 2025

kazcw marked this pull request as ready for review March 11, 2025 09:22

kazcw requested review from Frizi, farmaazon, vitvakatu, AdRiley, PabloBuchu, indiv0, somebody1234 and MrFlashAccount as code owners March 11, 2025 09:22

farmaazon approved these changes Mar 11, 2025

View reviewed changes

MrFlashAccount requested changes Mar 11, 2025

View reviewed changes

kazcw added 2 commits March 14, 2025 14:09

Merge remote-tracking branch 'origin/develop' into wip/kw/dashboard-docs

c0b81f7

Review

1558336

kazcw marked this pull request as draft March 14, 2025 16:44

kazcw marked this pull request as ready for review March 17, 2025 07:12

kazcw added 2 commits March 18, 2025 08:43

Merge remote-tracking branch 'origin/develop' into wip/kw/dashboard-docs

63ce199

Review

3a16d3c

kazcw requested a review from MrFlashAccount March 19, 2025 10:27

MrFlashAccount reviewed Mar 19, 2025

View reviewed changes

MrFlashAccount requested changes Mar 19, 2025

View reviewed changes

kazcw added 2 commits March 20, 2025 08:48

Merge remote-tracking branch 'origin/develop' into wip/kw/dashboard-docs

a13b0d2

Review

6be13fe

kazcw requested a review from MrFlashAccount March 20, 2025 09:56

MrFlashAccount approved these changes Mar 21, 2025

View reviewed changes

kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Mar 21, 2025

mergify bot merged commit 990ca19 into develop Mar 21, 2025
60 of 61 checks passed

mergify bot deleted the wip/kw/dashboard-docs branch March 21, 2025 13:33

jdunkerley added this to the 2025-Q1 Release milestone Mar 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use MarkdownEditor in dashboard #12462

Use MarkdownEditor in dashboard #12462

kazcw commented Mar 11, 2025 •

edited

Loading

github-actions bot commented Mar 11, 2025 •

edited

Loading

farmaazon Mar 11, 2025 •

edited

Loading

farmaazon Mar 11, 2025

MrFlashAccount Mar 11, 2025

farmaazon Mar 11, 2025

MrFlashAccount left a comment

MrFlashAccount Mar 11, 2025

MrFlashAccount Mar 11, 2025

kazcw Mar 14, 2025

MrFlashAccount Mar 18, 2025

MrFlashAccount Mar 11, 2025

MrFlashAccount Mar 19, 2025

farmaazon Mar 20, 2025

MrFlashAccount Mar 21, 2025

MrFlashAccount Mar 21, 2025

kazcw Mar 21, 2025 •

edited

Loading

MrFlashAccount Mar 19, 2025

MrFlashAccount left a comment

	/** Creates a React component wrapping a Vue component. */
	/** Creates a Lazy React component wrapping a Vue component. */

Use MarkdownEditor in dashboard #12462

Use MarkdownEditor in dashboard #12462

Conversation

kazcw commented Mar 11, 2025 • edited Loading

Pull Request Description

Important Notes

Checklist

github-actions bot commented Mar 11, 2025 • edited Loading

🧪 Storybook is successfully deployed!

📊 Dashboard:

farmaazon Mar 11, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kazcw Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrFlashAccount left a comment

Choose a reason for hiding this comment

kazcw commented Mar 11, 2025 •

edited

Loading

github-actions bot commented Mar 11, 2025 •

edited

Loading

farmaazon Mar 11, 2025 •

edited

Loading

kazcw Mar 21, 2025 •

edited

Loading