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

[Bug]: "Stream lookup": "Get lookup fields" and "Get fields" are flipped #3157

Closed
gertwieland opened this issue Aug 12, 2023 · 9 comments · Fixed by #3905
Closed

[Bug]: "Stream lookup": "Get lookup fields" and "Get fields" are flipped #3157

gertwieland opened this issue Aug 12, 2023 · 9 comments · Fixed by #3905
Assignees
Labels
Milestone

Comments

@gertwieland
Copy link

Apache Hop version?

2.5

Java version?

openjdk 11.0.19 2023-04-18 OpenJDK Runtime Environment OpenLogic-OpenJDK (build 11.0.19+7-adhoc..jdk11u) OpenJDK 64-Bit Server VM OpenLogic-OpenJDK (build 11.0.19+7-adhoc..jdk11u, mixed mode)

Operating system

Windows

What happened?

In the "Stream Lookup" field, I would expect the "Get Lookup fields" to pull the "Lookup fields", but it doesn't. Instead, it pulls the "Fields to retrieve".

But the "Get Fields" button pulls the "Lookup fields".

I think this should be the opposite.

image

Issue Priority

Priority: 3

Issue Component

Component: Hop Gui

@harrisward
Copy link

I think its right but a little confusion as the Get Lookup Fields Means "Get the fields to lookup from the Lookup Transform". Maybe the names of the buttons could be a bit better? Like "Get Key Fields" for the look up values and "Get Lookup Fields" for the fields to return to the primary transform

@hansva
Copy link
Contributor

hansva commented Aug 24, 2023

I agree that the naming is confusing.
what do you think about the following:
Get fields -> Get return fields
Get lookup fields -> get lookup key fields

?

@hansva
Copy link
Contributor

hansva commented Aug 24, 2023

.take-issue

@gertwieland
Copy link
Author

gertwieland commented Aug 24, 2023

I think the "Get lookup fields" should refer to the "LookupField" column in the upper part.

Then, in the lower part, it reads "Specify fields to retrieve". Maybe the button could just say "Get fields to retrieve"?

My idea is that this way the text in the headers matches the text on the buttons.

@nadment
Copy link
Contributor

nadment commented Apr 30, 2024

My proposal:
Get fields -> Get key fields
Get lookup fields -> Get fields to retrieve

@usbrandon
Copy link
Contributor

I agree with that and also adding the word 'key' "Get key fields" to let it contextualize what happens.

@nadment
Copy link
Contributor

nadment commented May 3, 2024

It's a little clearer this way ?

image

@usbrandon
Copy link
Contributor

"Get key fields" and "Get return fields". I would just make the case match, but it is much clearer.

@hansva
Copy link
Contributor

hansva commented May 7, 2024

Has been aligned:
image

hansva added a commit that referenced this issue May 7, 2024
Best understanding Stream lookup transform "Get fields" button #3157
@hansva hansva added this to the 2.9 milestone May 7, 2024
@hansva hansva assigned nadment and unassigned hansva May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants