-
Notifications
You must be signed in to change notification settings - Fork 21
internal: Floating recording controls #646
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
1870496
9f75f19
efaf7c2
2f5e165
e98a85d
02f9993
b40d31b
e9f7357
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 |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import { css } from '@emotion/react' | ||
| import { | ||
| DragHandleDots2Icon, | ||
| MinusCircledIcon, | ||
| StopIcon, | ||
| } from '@radix-ui/react-icons' | ||
| import { BrowserWindowConstructorOptions } from 'electron' | ||
| import { useEffect, useState } from 'react' | ||
|
|
||
| import { stopRecording } from '@/views/Recorder/Recorder.utils' | ||
| import { RecorderState } from '@/views/Recorder/types' | ||
|
|
||
| import { SubWindow } from './SubWindow' | ||
| import { Button } from './primitives/Button' | ||
|
|
||
| interface RecordingControlWindowProps { | ||
| state: RecorderState | ||
| } | ||
|
|
||
| const windowOptions: BrowserWindowConstructorOptions = { | ||
| width: 320, | ||
| height: 32, | ||
| alwaysOnTop: true, | ||
| resizable: false, | ||
| fullscreenable: false, | ||
| minimizable: false, | ||
| maximizable: false, | ||
| frame: false, | ||
| title: 'Recording toolbar', | ||
| useContentSize: true, | ||
| } | ||
|
|
||
| export function RecordingControlWindow({ state }: RecordingControlWindowProps) { | ||
| const [isOpen, setIsOpen] = useState(false) | ||
|
|
||
| const handleStopRecording = () => { | ||
| stopRecording() | ||
| } | ||
|
|
||
| const handleClose = () => { | ||
| setIsOpen(false) | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| console.log('RecordingControlWindow state', state) | ||
| setIsOpen(state === 'recording') | ||
| }, [state]) | ||
|
|
||
| if (!isOpen) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <SubWindow options={windowOptions} onClose={handleClose}> | ||
| <div | ||
| css={css` | ||
| box-sizing: border-box; | ||
| height: 100%; | ||
| display: flex; | ||
| align-items: center; | ||
| user-select: none; | ||
| gap: 8px; | ||
| padding: 4px 8px; | ||
| `} | ||
| > | ||
| <div | ||
| css={css` | ||
| app-region: drag; | ||
| font-size: var(--studio-font-size-1); | ||
|
|
||
| &:before { | ||
| content: ''; | ||
| display: inline-block; | ||
| width: 8px; | ||
| height: 8px; | ||
| margin-right: 4px; | ||
| background-color: #f00; | ||
| border-radius: 50%; | ||
| animation: pulse 2s ease-in-out infinite; | ||
|
|
||
| @keyframes pulse { | ||
| 0% { | ||
| transform: scale(1); | ||
| opacity: 1; | ||
| } | ||
| 50% { | ||
| transform: scale(1.2); | ||
| opacity: 0.5; | ||
| } | ||
| 100% { | ||
| transform: scale(1); | ||
| opacity: 1; | ||
| } | ||
| } | ||
| } | ||
| `} | ||
| > | ||
| Recording | ||
| </div> | ||
| <Button size="1" onClick={handleStopRecording}> | ||
| <StopIcon /> Stop | ||
| </Button> | ||
| <Button size="1" onClick={handleClose}> | ||
| <MinusCircledIcon /> Close | ||
| </Button> | ||
| <div | ||
| css={css` | ||
| app-region: drag; | ||
| display: flex; | ||
| justify-content: flex-end; | ||
| flex-grow: 1; | ||
| `} | ||
| > | ||
| <DragHandleDots2Icon /> | ||
| </div> | ||
| </div> | ||
| </SubWindow> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import createCache from '@emotion/cache' | ||
| import { CacheProvider, Global } from '@emotion/react' | ||
| import { BrowserWindowConstructorOptions } from 'electron' | ||
| import { PropsWithChildren, useEffect, useState } from 'react' | ||
| import { createPortal } from 'react-dom' | ||
|
|
||
| import { globalStyles } from '@/globalStyles' | ||
|
|
||
| import { Theme } from '../primitives/Theme' | ||
|
|
||
| interface SubWindowProps { | ||
| options: BrowserWindowConstructorOptions | ||
| onClose?: () => void | ||
| } | ||
|
|
||
| export function SubWindow({ | ||
|
Collaborator
Author
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. This approach is heavily inspired by https://pietrasiak.com/creating-multi-window-electron-apps-using-react-portals with some omissions (for example, it's currently not possible to watch for changes in window options - not sure if it's needed). It should make it much easier to work with multiple windows (e.g. render validator in a separate window rather than in a modal).
Collaborator
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. I personally don't find the points on why to avoid the default way of implementing multiple windows in electron that strong 🤔 (for example using IPC if needed doesn't see that negative since you already make use of it for main <-> renderer communication) I guess it's debatable but I wonder if there are more things that are going to bite us eventually if we use this approach for everything outside of the overlay 🤔
Collaborator
Author
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.
I can see a few points:
What are the benefits of the default way?
Collaborator
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.
Being able to use themes, I suppose. 🤔 But I think this is worth a shot. The DX looks pretty sweet and it's nice that state can be easily synced between windows. Doing this the "intended" way means that we'd have to open a window, wait for
Collaborator
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. I'm certainly missing knowledge here so my comment is directly tied to the points in the blog post, specifically the concern is regarding the caveat section:
Hopefully nothing major comes up if we apply it more broadly Beside that, I agree that is worth a try 🙌 |
||
| children, | ||
| options, | ||
| onClose, | ||
| }: PropsWithChildren<SubWindowProps>) { | ||
| const [id] = useState(crypto.randomUUID()) | ||
| const [subWindow] = useState<Window | null>(() => | ||
| window.open('about:blank', '_blank', JSON.stringify({ id, options })) | ||
| ) | ||
|
|
||
| useEffect(() => { | ||
| return () => { | ||
| if (subWindow) { | ||
| subWindow.close() | ||
| } | ||
| } | ||
| }, [subWindow]) | ||
going-confetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| useEffect(() => { | ||
| return window.studio.ui.onCloseWindow((windowId) => { | ||
| if (id !== windowId) { | ||
| return | ||
| } | ||
|
|
||
| onClose?.() | ||
| }) | ||
| }, [onClose, id]) | ||
|
|
||
| if (subWindow === null) { | ||
| return null | ||
| } | ||
|
|
||
| const subWindowCache = createCache({ | ||
| key: 'ksix-studio', | ||
| container: subWindow.document.head, | ||
| }) | ||
|
|
||
| return createPortal( | ||
| <CacheProvider value={subWindowCache}> | ||
| <Global styles={globalStyles} /> | ||
|
Comment on lines
+54
to
+55
Collaborator
Author
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. Unfortunately, Radix Themes won't work here (more context in this discussion). For this feature, it's not a big problem - I'll follow @allansson's approach from the browser branch - but it means that we can't easily use it to render request inspector/validator just yet :/ |
||
| <Theme root={true} includeColors /> | ||
| {children} | ||
| </CacheProvider>, | ||
| subWindow.document.body | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './SubWindow' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export type Feature = 'dummy-feature' | ||
| export type Feature = 'dummy-feature' | 'floating-recording-controls' |
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.
Didn't find a way to open with at a fixed height, while keeping it centered horizontally (
xis required ifyis set). I'll try to find a solution in the follow-up PRThere 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.
If we consider remembering the window position, we can reuse this function to keep track of x and y
k6-studio/src/main.ts
Lines 644 to 645 in f1f035a