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

Move network variables to store #286

Merged
merged 10 commits into from
Jul 1, 2021
Merged

Move network variables to store #286

merged 10 commits into from
Jul 1, 2021

Conversation

gotdairyya
Copy link
Contributor

@gotdairyya gotdairyya commented Jun 29, 2021

Closes #281

This change will make it possible to switch between queries without refreshing the page.
It will also show all possible options for large networks because it will pull the values from the original network.

@gotdairyya
Copy link
Contributor Author

This code mostly works, there is something going on within the FilterOverlay component where I am calculating the node and edge variables using AQL only if the network was subsetted.

The issue seems to be that the state is empty. This is the error I see: Property 'setLargeNetworkAttributeValues' does not exist on type '{}'

I have a feeling that it's due to async loading, but would like to discuss after you take a look

@gotdairyya gotdairyya marked this pull request as ready for review June 30, 2021 14:00
@gotdairyya gotdairyya requested a review from JackWilb June 30, 2021 14:00
Copy link
Member

@JackWilb JackWilb left a comment

Choose a reason for hiding this comment

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

I think there are some edge cases that need to be considered, but overall, this works great!

src/types.ts Outdated Show resolved Hide resolved
src/store/index.ts Outdated Show resolved Hide resolved
src/components/FilterOverlay.vue Outdated Show resolved Hide resolved
src/components/FilterOverlay.vue Show resolved Hide resolved
@JackWilb
Copy link
Member

Also, I didn't test this locally. I think we should test an example of 1 node table and 2 or more node tables to make sure that expansion works

src/store/index.ts Outdated Show resolved Hide resolved
JackWilb and others added 5 commits June 30, 2021 13:35
@gotdairyya
Copy link
Contributor Author

gotdairyya commented Jul 1, 2021

Also, I didn't test this locally. I think we should test an example of 1 node table and 2 or more node tables to make sure that expansion works

I tested with the Paul Revere dataset. It is pulling from the second table.

Also, I am getting a bug with nodes > 1, and edges > 1 in the dropdown.

To Do:

  • Get values for all node tables
  • Address issue with dropdown logic

Was causing issues because you can't map with an empty string
@gotdairyya gotdairyya requested a review from JackWilb July 1, 2021 18:51
@gotdairyya gotdairyya merged commit ebf951e into master Jul 1, 2021
@gotdairyya gotdairyya deleted the network-variables branch July 1, 2021 21:41
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.

ConnectivityMatrix query autofill options
2 participants