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

N8n 4040 data mapping #3708

Merged
merged 121 commits into from
Jul 20, 2022
Merged

N8n 4040 data mapping #3708

merged 121 commits into from
Jul 20, 2022

Conversation

mutdmour
Copy link
Contributor

No description provided.

Copy link
Contributor

@MiloradFilipovic MiloradFilipovic left a comment

Choose a reason for hiding this comment

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

Nice job! 🙌 Really looking forward to this feature.
I found a few nitpicks and possible refactor possibilities so once that is discussed/addressed this is good to go as far as I am concerned.

Some more stuff unrelated to code:

  • Personally, I expect from dragged value to overwrite current expression, not append to it so I find it confusing and a bit annoying that I have to go to editor and remove previous values. This is especially confusing since most of the time, only a single value (data pill) is visible in input field.
  • Just found out that combine field in the IF node accepts expressions. Haven't found anything regarding possible values other that ALL and ANY. Is this a bug?
    if_node_combine

@mutdmour
Copy link
Contributor Author

@MiloradFilipovic

Personally, I expect from dragged value to overwrite current expression, not append to it so I find it confusing and a bit annoying that I have to go to editor and remove previous values. This is especially confusing since most of the time, only a single value (data pill) is visible in input field.

I agree with you.. discussed this a lot but Max is confident in this.

Just found out that combine field in the IF node accepts expressions. Haven't found anything regarding possible values other that ALL and ANY. Is this a bug?

not a bug.. can be disabled directly in if node definition if it's not supposed to

@janober janober merged commit 577c73e into master Jul 20, 2022
@janober janober deleted the n8n-4040-data-mapping branch July 20, 2022 11:32
@janober
Copy link
Member

janober commented Jul 20, 2022

Amazing! Just got merged.

@janober janober added the Upcoming Release Will be part of the upcoming release label Jul 20, 2022
@janober
Copy link
Member

janober commented Jul 20, 2022

Got released with n8n@0.187.1

@janober janober removed the Upcoming Release Will be part of the upcoming release label Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants