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.

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

feat(editor): Make highlighted data pane floating #10638

Merged
merged 14 commits into from
Sep 4, 2024

Conversation

burivuhster
Copy link
Contributor

@burivuhster burivuhster commented Sep 2, 2024

Summary

Make annotation panel floating to save more canvas space

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/AI-301/make-highlighted-data-pane-floating

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)

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Sep 2, 2024
@burivuhster burivuhster marked this pull request as ready for review September 2, 2024 15:44
Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

Looks and works as expected but please use Vue 3 script setup syntax for new components 🙏

position: absolute;
bottom: 0;
right: var(--spacing-xl);
transform: translate(0, 100%);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we positioning it using transform? 🤔 Wouldn't this be easier?

.container {
	z-index: 1;
	position: absolute;
	right: var(--spacing-xl);
	top: var(--spacing-3xl);
	max-height: calc(100vh - 250px);
	width: 250px;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parent container's height is not fixed, and may grow because of text wrapping e.g. when viewport is extremely narrow or enlarged font sizes. It seems it's safer to rely on parent's bottom coordinates.

}
</style>

<style lang="scss" scoped>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to mix scoped and module styles? Why couldn't we just use module?

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 moved all styles to module, but had to add tags-container class onto the element, because it is defined globally in other files. Would be nice to refactor, but out of the scope of this PR.

@OlegIvaniv
Copy link
Contributor

@burivuhster Also just noticed: is it expected that I would see both floating and regular panel?
CleanShot 2024-09-03 at 09 07 37

@burivuhster burivuhster force-pushed the ai-301-make-highlighted-data-pane-floating branch from bfd9a3e to b17bab3 Compare September 3, 2024 09:07
@burivuhster burivuhster changed the title feat(core): Make highlighted data pane floating feat(editor): Make highlighted data pane floating Sep 3, 2024
Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

Just two linting and warnings and should be good to go!

@burivuhster
Copy link
Contributor Author

I've fixed onClickOutside when user clicks on canvas and also some vue prop warning as well

Copy link
Contributor

@OlegIvaniv OlegIvaniv left a comment

Choose a reason for hiding this comment

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

LGTM 👌

@burivuhster
Copy link
Contributor Author

Thanks, Oleg!

Copy link

cypress bot commented Sep 4, 2024

n8n    Run #6729

Run Properties:  status check passed Passed #6729  •  git commit a14644d87a: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 burivuhster 🗃️ e2e/*
Project n8n
Branch Review ai-301-make-highlighted-data-pane-floating
Run status status check passed Passed #6729
Run duration 04m 52s
Commit git commit a14644d87a: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 burivuhster 🗃️ e2e/*
Committer Eugene Molodkin
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 422
View all changes introduced in this branch ↗︎

Copy link
Contributor

github-actions bot commented Sep 4, 2024

✅ All Cypress E2E specs passed

@burivuhster burivuhster merged commit 8b5c333 into master Sep 4, 2024
33 checks passed
@burivuhster burivuhster deleted the ai-301-make-highlighted-data-pane-floating branch September 4, 2024 11:11
MiloradFilipovic added a commit that referenced this pull request Sep 5, 2024
* master:
  refactor(RabbitMQ Trigger Node): Improve type-safety, add tests, and fix issues with manual triggers (#10663)
  feat(editor): Add support for nodes with multiple main inputs in new canvas (no-changelog) (#10659)
  fix(editor): Set minimum zoom to 0 to allow fitting very large workflows in new canvas (no-changelog) (#10666)
  feat(editor): Change selection to be default canvas behaviour (no-changelog) (#10668)
  feat: More hints to nodes  (#10565)
  fix(editor): Fix opening executions tab from a new, unsaved workflow (#10652)
  fix(AI Agent Node): Fix tools agent when using memory and Anthropic models (#10513)
  feat(editor): Make highlighted data pane floating (#10638)
  fix(editor): Fix workflow loading after switching to executions view in new canvas (no-changelog) (#10655)
  refactor(benchmark): Separate cloud env provisioning from running benchmarks (#10657)
  feat(core): Implement wrapping of regular nodes as AI Tools (#10641)
  refactor(editor): Remove Trial logic in personalization modal and port to script setup (#10649)
  fix(core): Declutter webhook insertion errors (#10650)
  feat: Reintroduce collaboration feature (#10602)
  feat(benchmark): Add scenario for expressions with Set node (#10647)
  feat(benchmark): Add benchmark scenario for binary files (#10648)
  build: Add `reset` script (#10627)
  feat(editor): Overhaul node insert position computation in new canvas (no-changelog) (#10637)
@github-actions github-actions bot mentioned this pull request Sep 5, 2024
@janober
Copy link
Member

janober commented Sep 5, 2024

Got released with n8n@1.58.0

riascho pushed a commit that referenced this pull request Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants