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

Auto add suggested field names for csv import vGHC for OSD #548

Conversation

DariaNico
Copy link
Contributor

@DariaNico DariaNico commented Oct 1, 2021

PR to address issue #536 for Open Source Day hackathon/vGHC

config.pairs is not populated when setting columnKey, thus _find function always returns undefined, meaning columnKey is always set to null. This fix intentionally makes a pair array with tableColumns, thus allowing _find' to work, thus allowing columnKey` to be set.

@DariaNico DariaNico changed the title OSD vGHC Add suggested field names for csv import Auto add suggested field names for csv import Oct 1, 2021
@DariaNico DariaNico marked this pull request as ready for review October 1, 2021 23:13
@DariaNico
Copy link
Contributor Author

Screen.Recording.2021-10-01.at.6.21.50.PM.mov

@DariaNico DariaNico changed the title Auto add suggested field names for csv import Auto add suggested field names for csv import vGHC for OSD Oct 1, 2021
@notsidney notsidney merged commit 5c11185 into rowyio:develop Oct 7, 2021
@notsidney
Copy link
Contributor

Hi @DariaNico, thanks for your contribution! I noticed that this solution doesn’t let you continue to step 2, and doesn’t end up writing the column pairs to the config.

columnKey tells MultiSelect which column the user has selected. Because this was derived from whatever matches in tableColumns, the user couldn’t change which column was selected. The handleChange function writes the user’s selection in config.pairs, so columnKey must be derived from this.

Suggesting the column match is better suited right after the user states they want to import a CSV column, so I moved the logic there (lines 108–119)

You can review the fix in this commit: 796f4f1

@notsidney notsidney added this to the v2.1 milestone Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants