Repository navigation
feat(desktop): new-workspace screen experiment with sample prompts #5907
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
Changes from all commits
718cf3e
1dc05be
22ae32b
1a34f55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -141,6 +141,11 @@ interface MarkdownEditorProps { | |||||||||||||||||||||||||||||||||||||||
| className?: string; | ||||||||||||||||||||||||||||||||||||||||
| editorClassName?: string; | ||||||||||||||||||||||||||||||||||||||||
| onModEnter?: () => void; | ||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||
| * If provided, plain Enter fires this instead of inserting a newline | ||||||||||||||||||||||||||||||||||||||||
| * (Shift+Enter still breaks the line). Composer-style editors only. | ||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||
| onEnterSubmit?: () => void; | ||||||||||||||||||||||||||||||||||||||||
| /** If provided, enables @-mention file search for the editor. */ | ||||||||||||||||||||||||||||||||||||||||
| searchFiles?: FileMentionSearchFn; | ||||||||||||||||||||||||||||||||||||||||
| /** If provided, pasted file items (e.g. clipboard images) are forwarded here. */ | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -203,6 +208,7 @@ export function MarkdownEditor({ | |||||||||||||||||||||||||||||||||||||||
| className, | ||||||||||||||||||||||||||||||||||||||||
| editorClassName, | ||||||||||||||||||||||||||||||||||||||||
| onModEnter, | ||||||||||||||||||||||||||||||||||||||||
| onEnterSubmit, | ||||||||||||||||||||||||||||||||||||||||
| searchFiles, | ||||||||||||||||||||||||||||||||||||||||
| onPasteFiles, | ||||||||||||||||||||||||||||||||||||||||
| features, | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -218,6 +224,8 @@ export function MarkdownEditor({ | |||||||||||||||||||||||||||||||||||||||
| searchFilesRef.current = searchFiles; | ||||||||||||||||||||||||||||||||||||||||
| const onPasteFilesRef = useRef(onPasteFiles); | ||||||||||||||||||||||||||||||||||||||||
| onPasteFilesRef.current = onPasteFiles; | ||||||||||||||||||||||||||||||||||||||||
| const onEnterSubmitRef = useRef(onEnterSubmit); | ||||||||||||||||||||||||||||||||||||||||
| onEnterSubmitRef.current = onEnterSubmit; | ||||||||||||||||||||||||||||||||||||||||
| const editorRef = useRef<Editor | null>(null); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const urlPolicy = useInlineUrlPolicy(); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -353,6 +361,15 @@ export function MarkdownEditor({ | |||||||||||||||||||||||||||||||||||||||
| onModEnter?.(); | ||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||
| onEnterSubmitRef.current && | ||||||||||||||||||||||||||||||||||||||||
| event.key === "Enter" && | ||||||||||||||||||||||||||||||||||||||||
| !event.shiftKey && | ||||||||||||||||||||||||||||||||||||||||
| !event.altKey | ||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||
| onEnterSubmitRef.current(); | ||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+364
to
+372
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: ProseMirror is designed to ignore key events during IME composition to prevent them from interfering with the browser's native handling of composition sessions [1][2]. In its core implementation, ProseMirror's Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Locate MarkdownEditor file and related package files"
fd -a 'MarkdownEditor\.tsx|package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json' . | sed 's#^\./##' | head -80
echo
echo "MarkdownEditor outline"
ast-grep outline apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx --view expanded || true
echo
echo "Relevant MarkdownEditor sections"
wc -l apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
sed -n '200,390p' apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsx
echo
echo "Search for tiptap/prosemirror imports and usages"
rg -n "tiptap|prosemirror|handleKeyDown|onEnterSubmit|isComposing|ProseMirror" apps/desktop/src/renderer/components/MarkdownEditor apps/desktop -g '*.{ts,tsx}' -g 'package.json' -g 'pnpm-lock.yaml' -g 'yarn.lock' -g 'package-lock.json' | head -200Repository: superset-sh/superset Length of output: 40431 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Lockfile/package manager"
git ls-files | rg '(^|/)package\.json$|(^|/)pnpm-lock\.yaml$|(^|/)yarn\.lock$|(^|/)package-lock\.json$' | head -20
echo
echo "Desktop package dependencies"
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('apps/desktop/package.json','utf8'));
for (const section of ['dependencies','devDependencies']) {
console.log(`\n[${section}]`);
for (const [k,v] of Object.entries(p[section]||{}).filter(([k]) => k.includes('tiptap') || k.includes('prosemirror') || k.includes('electron'))) {
console.log(`${k}: ${v}`);
}
}
JS
echo
echo "Root lock snippets for tiptap/prosemirror/electron"
for f in pnpm-lock.yaml yarn.lock package-lock.json; do
[ -f "$f" ] || continue
echo "--- $f ---"
rg -n "`@tiptap`|prosemirror|electron(-main|preload)?|electron(\@|:)|desktop app|electron|`@electron`" "$f" | head -120 || true
done
echo
echo "KeyboardHandler implementation"
sed -n '100,135p' apps/desktop/src/renderer/components/MarkdownEditor/MarkdownEditor.tsxRepository: superset-sh/superset Length of output: 3265 🌐 Web query:
💡 Result: In Tiptap (built on ProseMirror), handling keyboard events during an IME composition session requires caution because browsers fire events in complex sequences that can cause input corruption if intercepted improperly [1][2]. To properly manage handleKeyDown in a Tiptap or ProseMirror context while respecting composition: 1. Use event.isComposing: The standard way to detect if a keyboard event is occurring during an IME composition session is to check the read-only isComposing property of the KeyboardEvent [3]. In your handleKeyDown plugin prop, you should generally ignore events where event.isComposing is true to avoid interfering with the native composition process [2]. 2. Understand ProseMirror's Role: ProseMirror's core view logic already includes internal guards—such as inOrNearComposition—to prevent the editor from attempting to handle key events that occur during or immediately around an IME composition session [1]. Interfering with this by trying to force-handle or block events can lead to duplicated characters, incorrect text placement, or broken composition flow [2]. 3. Implementation Example: When writing a custom extension that uses a ProseMirror plugin to intercept keys, you can implement the check as follows: import { Extension } from ' Citations:
🌐 Web query:
💡 Result: In Tiptap (which is built on ProseMirror), composition events and the isComposing state are handled primarily at the ProseMirror level rather than within Tiptap's core source code [1][2]. When you implement custom keyboard handling via Tiptap extensions—such as using the handleKeyDown prop—it is important to respect the browser's native composition session to avoid issues with IME (Input Method Editor) input, which is common for languages like Chinese, Japanese, and Korean [1][2]. The KeyboardEvent.isComposing property is a standard Web API that indicates whether a key event occurs during a composition session (i.e., after compositionstart and before compositionend) [3]. In ProseMirror's event handling architecture, this is used to prevent the editor from prematurely processing key inputs that are intended to be part of an ongoing composition [4][2]. If you are using handleKeyDown in a Tiptap extension, you can check the isComposing property on the event object to decide whether to bypass your custom logic [5][2]. A common pattern looks like this: handleKeyDown(view, event) { if (event.isComposing) { return false; // Let the browser/ProseMirror handle the event during composition } // Your custom logic here return true; } ProseMirror internally manages composition events (via compositionstart, compositionupdate, and compositionend) to ensure the document state remains consistent with the DOM [4][2]. Manually interfering with these events is generally discouraged, as the browser must handle them normally for composition to function correctly [2]. Issues involving composition in Tiptap are often inherited from ProseMirror's input handling, where special care is required to ensure that editor state updates do not interrupt or prematurely finalize a composition session [1][4]. Citations:
Check Even though Ti/Tiptap uses ProseMirror under the hood, this custom 🐛 Proposed guard if (
onEnterSubmitRef.current &&
event.key === "Enter" &&
+ !event.isComposing &&
!event.shiftKey &&
!event.altKey
) {
onEnterSubmitRef.current();
return true;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||
| handlePaste: (_, event) => { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { PromptInputProvider } from "@superset/ui/ai-elements/prompt-input"; | ||
| import { createFileRoute } from "@tanstack/react-router"; | ||
| import { NewWorkspaceScreen } from "renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen"; | ||
| import { DashboardNewWorkspaceDraftProvider } from "renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceDraftContext"; | ||
|
|
||
| export const Route = createFileRoute( | ||
| "/_authenticated/_dashboard/new-workspace/", | ||
| )({ | ||
| validateSearch: ( | ||
| search: Record<string, unknown>, | ||
| ): { projectId?: string } => ({ | ||
| projectId: | ||
| typeof search.projectId === "string" ? search.projectId : undefined, | ||
| }), | ||
| component: NewWorkspacePage, | ||
| }); | ||
|
|
||
| /** | ||
| * Experiment test arm (new-workspace-screen): the create surface as a real | ||
| * route. Store opens are redirected here by DashboardNewWorkspaceModal. | ||
| */ | ||
| function NewWorkspacePage() { | ||
| const { projectId } = Route.useSearch(); | ||
| return ( | ||
| <DashboardNewWorkspaceDraftProvider onClose={() => {}}> | ||
| <PromptInputProvider> | ||
| <NewWorkspaceScreen isOpen preSelectedProjectId={projectId ?? null} /> | ||
| {/* Window-drag surface replacing the hidden TopBar's drag region. */} | ||
| <div className="drag absolute inset-x-0 top-0 z-50 h-12" /> | ||
| </PromptInputProvider> | ||
| </DashboardNewWorkspaceDraftProvider> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { PromptInputProvider } from "@superset/ui/ai-elements/prompt-input"; | ||
| import { NewWorkspaceScreen } from "renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/NewWorkspaceScreen"; | ||
| import { DashboardNewWorkspaceDraftProvider } from "renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/DashboardNewWorkspaceDraftContext"; | ||
|
|
||
| /** | ||
| * Experiment test arm (new-workspace-screen): replaces the "No workspaces yet" | ||
| * empty state with the create screen itself. With zero workspaces there is | ||
| * nothing behind it, so it is pinned — close actions (Escape) are no-ops. | ||
| * Store opens are not handled here: DashboardNewWorkspaceModal redirects them | ||
| * to the /new-workspace route. | ||
| */ | ||
| export function NewWorkspaceEmptyScreen() { | ||
| return ( | ||
| <DashboardNewWorkspaceDraftProvider onClose={() => {}}> | ||
| <PromptInputProvider> | ||
| <NewWorkspaceScreen isOpen preSelectedProjectId={null} /> | ||
| </PromptInputProvider> | ||
| </DashboardNewWorkspaceDraftProvider> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { NewWorkspaceEmptyScreen } from "./NewWorkspaceEmptyScreen"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: superset-sh/superset
Length of output: 24656
🌐 Web query:
Node.js fs.writeFile exclusive flag EEXIST race condition💡 Result:
Yes, using the wx flag with fs.writeFile (or the corresponding file system method) is the correct, recommended way to prevent race conditions when creating files [1][2]. The 'wx' flag stands for "write" and "exclusive" (x) [3]. When this flag is used, the underlying operating system performs the check for the file's existence and the file creation as a single, atomic operation [1]. This atomicity is critical because it eliminates the "time-of-check to time-of-use" (TOCTOU) race condition that occurs if you attempt to manually check for a file's existence using methods like fs.exists (deprecated) or fs.stat before writing [4][1][5]. In those manual approaches, another process could potentially create the file in the small window of time between your check and your write command, leading to an accidental overwrite [1]. When you use the wx flag: 1. The operation will fail if the file already exists, and the error returned will have the code EEXIST [1][5]. 2. If the file does not exist, it will be created and written to atomically [1]. Important considerations: - Handle the error: Since 'wx' causes an error when the file exists, your code must explicitly catch and handle the EEXIST error code to respond appropriately (e.g., by logging a message or choosing a different filename) [1][5]. - Not a general lock: While the wx flag ensures atomic creation, it does not act as a general file lock for subsequent read/write operations [3]. If you need to perform multiple operations on the same file without other processes interfering, you should look into more robust file locking mechanisms or architectural patterns [3]. - Don't use existence checks first: Official Node.js documentation explicitly warns against using fs.access or similar checks to test for file accessibility before calling file system methods, as this introduces the very race condition that the flags are designed to avoid [4][5].
Citations:
Make collision handling atomic.
The
existsSync(target)probe and subsequentwriteFileare separate operations, so concurrent saves resolving to the same filename can race and one can be lost. Use an exclusive write (wx) loop that retries onEEXISTinstead of probing before each write.🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 164-167: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.promises.writeFile(
target,
Buffer.from(input.dataBase64, "base64"),
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Source: Linters/SAST tools