-
Notifications
You must be signed in to change notification settings - Fork 38.8k
feat(editor): Action/credentials tab in node settings in zoomed view (no-changelog) #17730
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
Conversation
- Add dedicated content panels for credential and action tabs in embedded canvas view - Fix credential tab visibility condition to use isDisplayingCredentials instead of parametersByTab.credential.length - Order tab panels as: action, credential, params, settings for logical workflow - Action tab displays resource/operation parameters using ParameterInputList - Credential tab displays NodeCredentials component with proper event handling 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
BundleMonFiles added (2)
Total files change +255.73KB Groups added (2)
Final result: ✅ View report in BundleMon website ➡️ |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Extracted from NodeCredentials.vue
@@ -848,10 +839,6 @@ onBeforeUnmount(() => { | |||
</template> | |||
|
|||
<style lang="scss"> | |||
// Hide notice(.ndv-connection-hint-notice) warning when node has output connection | |||
[data-has-output-connection='true'] .ndv-connection-hint-notice { |
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.
Moving to NodeSettings.vue so I don't have to replicate this for embedded NDV too. .ndv-connection-hint-notice
is set in ParameterInputList.vue which is inside of NodeSettings.vue, so the effect will remain the same.
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.
Extracting some code into useNodeCredentialOptions for reuse
Claude finished @autologie's task —— View job Analyzing your PR for the Action/Credentials tab feature... Todo List
Review SummaryThis is a well-structured PR that introduces separate Action and Credentials tabs in the Node Details View (NDV) for zoomed view. The implementation is clean and follows good architecture patterns. 🟢 StrengthsClean Architecture & Code Organization:
User Experience Improvements:
Implementation Quality:
🟡 Areas for ImprovementMinor Code Quality Issues:
Edge Cases:
🔴 Issues FoundTest Failure Analysis: Potential Issues:
🏆 Code Examples of ExcellenceThe action selection flow is particularly well implemented: // NodeSettings.vue:566-579 - Clean action selection with automatic flow
function handleSelectAction(params: INodeParameters) {
// Update parameters first
for (const [key, value] of Object.entries(params)) {
valueChanged({ name: `parameters.${key}`, value });
}
// Smart navigation to next required step
if (isDisplayingCredentials.value && !areAllCredentialsSet.value) {
onTabSelect('credential');
return;
}
if (parametersByTab.value.params.length > 0) {
onTabSelect('params');
}
} 📋 Recommendations
✅ Overall AssessmentThis is a high-quality implementation that successfully addresses the UX challenge of limited space in zoomed view. The code is clean, well-organized, and follows Vue 3 best practices. The failing test appears unrelated to your changes and shouldn't block the PR. Recommendation: Approve with minor suggestions addressed |
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.
cubic analysis
1 issue found across 17 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
packages/frontend/editor-ui/src/composables/useNodeCredentialOptions.ts
Outdated
Show resolved
Hide resolved
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.
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.
🚀
@MiloradFilipovic thank you, I think we didn't consider the case where auth field is conditional. I'll discuss this with David and Jake. The UI glitch is a known issue – we'll do an investigation. |
E2E Tests: n8n tests passed after 5m 50.8s Run Details
This message was posted automatically by
currents.dev | Integration Settings
|
…(no-changelog) (#17730) Co-authored-by: Claude <[email protected]>
…(no-changelog) (#17730) Co-authored-by: Claude <[email protected]>
Got released with |
Summary
To better fit with the limited width and height, we're re-organizing tabs in NDV in the zoomed view.
To try out, set
N8N_EXPERIMENTAL_MIN_ZOOM_NODE_SETTINGS_IN_CANVAS
to2
in local storage and zoom in the canvas.Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/SUG-94/zoomed-view-add-action-and-cred-tabs
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)