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

fix(editor): Never show Pinned Data Callout for Input Panel #12446

Merged
merged 3 commits into from
Jan 7, 2025
+30 −100
Merged

fix(editor): Never show Pinned Data Callout for Input Panel #12446

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1a816fe
Never show on input panel
CharlieKolb Jan 3, 2025
47df384
Update snapshot
CharlieKolb Jan 3, 2025
cffab03
Add tests
CharlieKolb Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/editor-ui/src/components/RunData.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,12 @@ defineExpose({ enterEditMode });
<template>
<div :class="['run-data', $style.container]" @mouseover="activatePane">
<N8nCallout
v-if="pinnedData.hasData.value && !editMode.enabled && !isProductionExecutionPreview"
v-if="
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the quick fix. Can you please add a test?

Copy link
Contributor Author

@CharlieKolb CharlieKolb Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This wasn't really a bug, but rather a change in (semi-)intended behavior.

I can add tests for the callout banner in general (we don't have any, only for the pin button), but it's probably best logistically to separate that into a tech debt ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

Luckily we already have good coverage for RunData. Can we simply add a unit test for this there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out the input panel has a snapshot test, I have pushed a change to update that. It will catch this showing up accidentally. (This also means the change was more or less intentional on our end back then, since we explicitly updated the snapshot, so just some miscommunication.)

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to have a clear check if there's no pinned data callout in panel, to avoid this bug being reintroduced. Because snapshots don't convey those requirements and get updated often without full consideration/review.

!isPaneTypeInput &&
pinnedData.hasData.value &&
!editMode.enabled &&
!isProductionExecutionPreview
"
theme="secondary"
icon="thumbtack"
:class="$style.pinnedDataCallout"
Expand Down
Loading