-
Notifications
You must be signed in to change notification settings - Fork 1.7k
bug:added restart dev button #2755
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
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,13 +7,15 @@ import { observer } from 'mobx-react-lite'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { motion } from 'motion/react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState } from 'react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Terminal } from './terminal'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { toast } from '@onlook/ui/sonner'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const TerminalArea = observer(({ children }: { children: React.ReactNode }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const editorEngine = useEditorEngine(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const terminalSessions = editorEngine.sandbox.session.terminalSessions; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const activeSessionId = editorEngine.sandbox.session.activeTerminalSessionId; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [terminalHidden, setTerminalHidden] = useState(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [isRestarting, setIsRestarting] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!terminalSessions.size) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -24,6 +26,23 @@ export const TerminalArea = observer(({ children }: { children: React.ReactNode | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const handleRestartDevServer = async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isRestarting) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsRestarting(true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const restartResponse = await editorEngine.sandbox.session.restartDevServer(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (restartResponse) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toast.success('Dev server restarting'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toast.error('Failed to restart dev server'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| toast.error('Error restarting dev server'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } finally { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setIsRestarting(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {terminalHidden ? ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -57,6 +76,17 @@ export const TerminalArea = observer(({ children }: { children: React.ReactNode | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </motion.span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="flex items-center gap-1"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <motion.div layout>{/* <RunButton /> */}</motion.div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Tooltip> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <TooltipTrigger asChild> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick={() => handleRestartDevServer()} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="h-9 w-9 flex items-center justify-center hover:text-foreground-hover text-foreground-tertiary hover:bg-accent/50 rounded-md border border-transparent" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Icons.Reload className={cn("h-4 w-4", isRestarting && "animate-spin")}/> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </button> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </TooltipTrigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <TooltipContent sideOffset={5} hideArrow>Restart dev server</TooltipContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Tooltip> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+79
to
+89
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. 🛠️ Refactor suggestion Disable during restart + a11y + avoid extra closure
- <TooltipTrigger asChild>
- <button
- onClick={() => handleRestartDevServer()}
- className="h-9 w-9 flex items-center justify-center hover:text-foreground-hover text-foreground-tertiary hover:bg-accent/50 rounded-md border border-transparent"
- >
- <Icons.Reload className={cn("h-4 w-4", isRestarting && "animate-spin")}/>
- </button>
- </TooltipTrigger>
+ <TooltipTrigger asChild>
+ <button
+ onClick={handleRestartDevServer}
+ disabled={isRestarting}
+ aria-label="Restart dev server"
+ aria-busy={isRestarting}
+ className={cn(
+ "h-9 w-9 flex items-center justify-center rounded-md border border-transparent",
+ "text-foreground-tertiary hover:text-foreground-hover hover:bg-accent/50",
+ "disabled:opacity-50 disabled:cursor-not-allowed disabled:pointer-events-none"
+ )}
+ >
+ <Icons.Reload
+ className={cn("h-4 w-4", isRestarting && "animate-spin")}
+ aria-hidden="true"
+ />
+ </button>
+ </TooltipTrigger>This also aligns with the PR description claiming the button is disabled during restart. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Tooltip> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <TooltipTrigger asChild> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Avoid per-component restart flags; centralize in store
If there are multiple restart entry points (e.g., CodeControls), a local
isRestartingcan allow concurrent restarts from other surfaces. Prefer a single observable flag oneditorEngine.sandbox.session(e.g.,isRestartingDevServer) or haverestartDevServer()self-gate reentry and expose status.🤖 Prompt for AI Agents