Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(editor): Popping logs out into a new window #13788

Merged
merged 31 commits into from
Mar 17, 2025
Merged

feat(editor): Popping logs out into a new window #13788

merged 31 commits into from
Mar 17, 2025
+459 −149

Conversation

autologie
Copy link
Contributor

@autologie autologie commented Mar 10, 2025 •
edited
Loading

Summary

This PR implements the feature to pop out the chat and logs pane at the bottom of canvas.
The feature is built on top of Google Chrome's experimental documentPictureInPicture API. For other browsers and insecure context which disallows the usage of this API, the button to pop out the pane is hidden.

Screencast.from.2025-03-12.09-29-16.webm

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/SUG-20/popping-logs-out-into-a-new-window

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

Sorry, something went wrong.

@n8n-assistant n8n-assistant bot added the n8n team Authored by the n8n team label Mar 10, 2025
@codecov Codecov
Copy link

codecov bot commented Mar 11, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.08458% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../components/CanvasChat/composables/usePiPWindow.ts 72.30% 18 Missing ⚠️
...editor-ui/src/components/CanvasChat/CanvasChat.vue 86.66% 12 Missing ⚠️
...s/frontend/editor-ui/src/stores/workflows.store.ts 66.66% 2 Missing ⚠️
...ontend/editor-ui/src/composables/useRunWorkflow.ts 0.00% 1 Missing ⚠️
...ges/frontend/editor-ui/src/composables/useToast.ts 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@autologie
hide return button
Loading
Loading status checks…
fe174e7
@autologie
refactor
Loading
Loading status checks…
7b11538
@autologie
add some tests
Loading
Loading status checks…
ef1a101
autologie
autologie commented Mar 12, 2025
View reviewed changes
packages/frontend/@n8n/design-system/src/components/N8nResizeWrapper/ResizeWrapper.vue
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file makes it possible to listen to the right window object when the resizer is used in a PiP window

packages/frontend/@n8n/design-system/src/components/N8nTooltip/Tooltip.vue Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file are needed to show tooltips properly in a PiP window.
I'm using provide/inject instead of props so that I don't have to modify individual usages of the tooltip component.

packages/frontend/editor-ui/src/components/CanvasChat/CanvasChat.test.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with product to remove the confirmation dialog of clearing the chat history. In the future we'll need to make the dialog work in PiP window as well. Right now it always shows in the main window even when the user is interacting in PiP window.

packages/frontend/editor-ui/src/composables/useClipboard.ts Outdated
isSupported,
// When the `copy()` method is invoked from inside of the document picture-in-picture (PiP) window, it throws the error "Document is not focused".
// Therefore, we disable copying features in the PiP window for now.
isSupported: computed(() => isSupported && !isInPiPWindow.value),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any usages of isSupported in the codebase, so it should be safe to change boolean to ComputedRef<boolean> here.

@autologie autologie marked this pull request as ready for review March 12, 2025 10:59
@autologie
fix formatting
Loading
Loading status checks…
48d55bf
@autologie
fix formatting
Loading
Loading status checks…
325e6c0
@autologie autologie requested a review from alexgrozav March 12, 2025 13:07
alexgrozav
alexgrozav reviewed Mar 13, 2025
View reviewed changes
Copy link
Member

@alexgrozav alexgrozav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few small remarks.

Great work on this! 🤩

@autologie autologie requested a review from alexgrozav March 14, 2025 09:14
@github-actions GitHub Actions
Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

@cypress Cypress
Copy link

cypress bot commented Mar 14, 2025 •
edited
Loading

n8n    Run #9783

Run Properties:  status check passed Passed #9783  •  git commit e9f40eccb7: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 autologie 🗃️ e2e/*
Project n8n
Branch Review sug-20-popout-logs-view
Run status status check passed Passed #9783
Run duration 04m 50s
Commit git commit e9f40eccb7: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 autologie 🗃️ e2e/*
Committer autologie
View all properties for this run ↗︎
Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 2
Tests that did not run due to a developer annotating a test with .skip  Pending 6
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 454
View all changes introduced in this branch ↗︎

@autologie
remove ineffective key
Loading
Loading status checks…
2db188a
@autologie autologie requested a review from alexgrozav March 14, 2025 13:43
alexgrozav
alexgrozav previously approved these changes Mar 14, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

⚠️ Some Cypress E2E specs are failing, please fix them before merging

@autologie
update e2e
Loading
Loading status checks…
9835540
@autologie autologie requested a review from alexgrozav March 14, 2025 15:06
@autologie
fix AI assistant floating button position
Loading
Loading status checks…
e9f40ec
@github-actions GitHub Actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@autologie autologie merged commit 4d04c22 into master Mar 17, 2025
36 checks passed
@autologie autologie deleted the sug-20-popout-logs-view branch March 17, 2025 09:50
@github-actions github-actions bot mentioned this pull request Mar 17, 2025
Merged
@janober
Copy link
Member

janober commented Mar 17, 2025

Got released with n8n@1.84.0

MrJasperTan pushed a commit to MrJasperTan/n8n that referenced this pull request Mar 17, 2025
@autologie
feat(editor): Popping logs out into a new window (n8n-io#13788)
57a3274
guillaumejacquart pushed a commit that referenced this pull request Mar 18, 2025
@autologie @guillaumejacquart
feat(editor): Popping logs out into a new window (#13788)
9526ab7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexgrozav alexgrozav

Assignees
No one assigned
Labels
n8n team Authored by the n8n team Released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@autologie @janober @alexgrozav