internal: Floating recording controls #646

going-confetti · 2025-04-10T14:20:26Z

Description

Add a floating recording control window

⚠️ Note: Because of some styling issues, this is hidden behind a feature toggle. It will be styled in the next PR

How to Test

Turn on the feature toggle
Start a recording
1. Verify that the controls are shown and you can drag them around
2. Verify that clicking on Stop stops the recording
3. Verify that clicking on Close closes the controls window without stopping the recording

Checklist

I have performed a self-review of my code.
I have added tests for my changes.
I have run linter locally (npm run lint) and all checks pass.
I have run tests locally (npm test) and all tests pass.
I have commented on my code, particularly in hard-to-understand areas.

Screenshots (if appropriate):

Related PR(s)/Issue(s)

#594

going-confetti · 2025-04-10T14:24:35Z

src/components/RecordingControlWindow.tsx

+  isOpen: boolean
+}
+
+const windowOptions: BrowserWindowConstructorOptions = {


Didn't find a way to open with at a fixed height, while keeping it centered horizontally (x is required if y is set). I'll try to find a solution in the follow-up PR

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

async function trackWindowState(browserWindow: BrowserWindow) {

const { width, height, x, y } = browserWindow.getBounds()

going-confetti · 2025-04-10T14:27:27Z

src/components/SubWindow/SubWindow.tsx

+  options: BrowserWindowConstructorOptions
+}
+
+export function SubWindow({


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).

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 🤔

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 can see a few points:

You need a separate entry point for each window as each window is its own React SPA, so you can't create windows ad-hoc

The portal approach is more declarative and offers a better DX (in my opinion)

The portal approach makes it very easy to render a component tree in a window, which means you can implement detachable panels (validator/request inspector) in a very simple way

What are the benefits of the default way?

What are the benefits of the default way?

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 React.createRoot to finish and then sync the state from when the window was docked. And on top of that we need to keep two views in sync.

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:

There are many other things, and I think I don’t even remember all of those now

Hopefully nothing major comes up if we apply it more broadly

Beside that, I agree that is worth a try 🙌

src/main.ts

going-confetti · 2025-04-10T14:31:43Z

src/components/SubWindow/SubWindow.tsx

+    <CacheProvider value={subWindowCache}>
+      <Global styles={globalStyles} />


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 :/

src/components/SubWindow/SubWindow.tsx

going-confetti · 2025-04-10T14:34:27Z

src/components/RecordingControlWindow.tsx

+            align-items: center;
+          `}
+        >
+          <StopIcon /> Stop recording


For this internal iteration of the feature, stop is the only supported action. What else would we like to see?

I'm thinking about creating groups and clearing already recorded requests - although the latter might not be ideal because we don't have show the request list in the controls window, so one would need to make a decision blindly.

Creating a new group and indicating which group the user is in could be a start.
Regarding deletion of things is something that we still need to tackle for the main view as well 🤔

Regarding deletion of things is something that we still need to tackle for the main view as well 🤔

Yes, I don't think deleting individual request in this control panel is feasible. I meant the existing Clear/Remove all requests from the recording functionality

…ntrol-draft

e-fisher

Have a few suggestions, feel free to ignore if they are already planned for next PR:

Can we move the floating window to one of the screen corners, so it's not in the way when starting recording?
Would be nice to remember last used position when starting new recording
Maybe add a button to close the floating window without stopping the recording? On smaller displays, the floating window could be getting in your way, and you'd have to keep moving it 🤔

Llandy3d · 2025-04-14T10:37:27Z

3. Maybe add a button to close the floating window without stopping the recording? On smaller displays, the floating window could be getting in your way, and you'd have to keep moving it 🤔

Maybe a minimize button to make it go away but still be retrievable 🤔
An evolution or quality of life improvement would also be to have shortcuts to do that operation 😄

going-confetti · 2025-04-14T13:51:01Z

@e-fisher @Llandy3d I responded to your suggestions in a single comment:

Can we move the floating window to one of the screen corners, so it's not in the way when starting recording?

I was thinking either this or at the top of the screen, but horizontally centered 👍

Would be nice to remember last used position when starting new recording

Certainly!

Maybe add a button to close the floating window without stopping the recording? On smaller displays, the floating window could be getting in your way, and you'd have to keep moving it 🤔

Good idea. I think the window will be much smaller in the end (we probably don't need all this text), but it's still worth it to have this option.

Maybe a minimize button to make it go away but still be retrievable

I would prefer it to be a non-minimizable control window (so more like GMeet screen sharing controls or QuickTime recording overlay). One idea is to add a toggle button to the Recorder UI to open it if the user dismissed it. I would probably tackle this in the next iteration of this feature.

…ntrol-draft

cristianoventura

LGTM!

I noticed the Close button isn't working ATM, but I assume it's intended since there'll be iterations coming up?

cristianoventura · 2025-04-16T15:12:08Z

src/components/RecordingControlWindow.tsx

+  isOpen: boolean
+}
+
+const windowOptions: BrowserWindowConstructorOptions = {


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

async function trackWindowState(browserWindow: BrowserWindow) {

const { width, height, x, y } = browserWindow.getBounds()

going-confetti · 2025-04-16T15:26:02Z

LGTM!

I noticed the Close button isn't working ATM, but I assume it's intended since there'll be iterations coming up?

Good catch! I fixed it 👀

cristianoventura

LGTM!

I was curious to see what was going to happen with the floating controls when you close the main window (as closing it doesn't really kill the application on macOS) but we currently have an issue in main that prevents the main window from being restored if you close it.

Ideally, the main window should always be kept when a recording is in progress. I made a ticket for us to address that issue: #667

internal: Floating recording controls

1870496

going-confetti commented Apr 10, 2025

View reviewed changes

going-confetti marked this pull request as ready for review April 10, 2025 14:34

going-confetti requested a review from a team as a code owner April 10, 2025 14:34

going-confetti requested review from cristianoventura, Llandy3d, allansson and e-fisher April 10, 2025 14:34

going-confetti added 2 commits April 11, 2025 11:32

handle closing from main

9f75f19

Merge remote-tracking branch 'origin/main' into internal/recording-co…

efaf7c2

…ntrol-draft

e-fisher reviewed Apr 14, 2025

View reviewed changes

going-confetti added 4 commits April 16, 2025 12:09

Merge remote-tracking branch 'origin/main' into internal/recording-co…

2f5e165

…ntrol-draft

remove comment

e98a85d

revert changes in package-lock

02f9993

update styles

b40d31b

cristianoventura previously approved these changes Apr 16, 2025

View reviewed changes

fix closing

e9f7357

going-confetti dismissed cristianoventura’s stale review via e9f7357 April 16, 2025 15:24

cristianoventura approved these changes Apr 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

internal: Floating recording controls #646

internal: Floating recording controls #646

going-confetti commented Apr 10, 2025 •

edited

Loading

going-confetti Apr 10, 2025

cristianoventura Apr 16, 2025

going-confetti Apr 10, 2025

Llandy3d Apr 10, 2025

going-confetti Apr 11, 2025

allansson Apr 11, 2025 •

edited

Loading

Llandy3d Apr 11, 2025

going-confetti Apr 10, 2025

going-confetti Apr 10, 2025

Llandy3d Apr 10, 2025

going-confetti Apr 11, 2025

e-fisher left a comment

Llandy3d commented Apr 14, 2025

going-confetti commented Apr 14, 2025 •

edited

Loading

cristianoventura left a comment

cristianoventura Apr 16, 2025

going-confetti commented Apr 16, 2025

cristianoventura left a comment

	async function trackWindowState(browserWindow: BrowserWindow) {
	const { width, height, x, y } = browserWindow.getBounds()

		<CacheProvider value={subWindowCache}>
		<Global styles={globalStyles} />

internal: Floating recording controls #646

Are you sure you want to change the base?

internal: Floating recording controls #646

Conversation

going-confetti commented Apr 10, 2025 • edited Loading

Description

How to Test

Checklist

Screenshots (if appropriate):

Related PR(s)/Issue(s)

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

allansson Apr 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e-fisher left a comment

Choose a reason for hiding this comment

Llandy3d commented Apr 14, 2025

going-confetti commented Apr 14, 2025 • edited Loading

cristianoventura left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

going-confetti commented Apr 16, 2025

cristianoventura left a comment

Choose a reason for hiding this comment

going-confetti commented Apr 10, 2025 •

edited

Loading

allansson Apr 11, 2025 •

edited

Loading

going-confetti commented Apr 14, 2025 •

edited

Loading