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

Improve node and edge filter options #289

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

kmcginnes
Copy link
Collaborator

@kmcginnes kmcginnes commented Mar 29, 2024

Description

Reduce filter config resets

We were manually reseting the filter options every time the schema nodes and edges change, which is triggered by expanding a node. This is because the schema may change when we get back the neighbors of the expanded node. Those new node or edge types should be added to the schema, and therefore the filter options.

However, we don't want to reset the filter options on expansion. So instead of storing the list of visible nodes and edges in the filter state I changed it to store the hidden items. This means the default state with all nodes and edges selected is an empty array.

We no longer need to update the stored filter options when the schema changes because of an expansion. Any new types will be selected automatically. Any unselected items will remain unselected.

The filters properly reset when the connection changes.

Sidebar badge for filters

There is a chance for user confusion with the filter reset change above.

Let's say you have filtered out all but one node type. Now you expand that node. Nothing in the UI will change since you have all the other node types hidden.

To make it more obvious that a filter is applied, I've added a badge to the filter sidebar button. It will show any time a node or edge is unselected.

Testing

I discovered the reason why testing the React hooks was causing Jest to fail to run the tests. The drag and drop library we add to the app through React context is exported by the /core/index.ts file. Simply importing from /core/index.ts would cause running the tests to fail, preventing us from properly testing the hook logic.

By changing how we import some of the types, and removing ConnectedProvider from index.ts writing tests for the hooks became much simpler.

I've also added some data generation helpers. They can create random data in general, and random schemas specifically. This helps in test setup for the majority of our logic, since the schema and config is typically the starting point for our logic.

Validation

Verify filters remain when expanding

  • Uncheck some filters for nodes and edges
  • Expand the selected node
  • See the hidden nodes and edges remain hidden

Verify filters reset when changing configs

  • Uncheck some filters for nodes and edges
  • Switch to a different connection
  • See that filters are reset to all selected

Verify filter badge

  • Badge should show when any edge or filter is unchecked
  • Badge should hide when all edges and filters are selected

CleanShot 2024-03-29 at 17 36 00@2x

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have run pnpm run checks to ensure code compiles and meets standards.
  • I have run pnpm run test to check if all tests are passing.
  • I've covered new added functionality with unit tests if necessary.

@kmcginnes kmcginnes marked this pull request as ready for review March 29, 2024 23:05
@xiazcy xiazcy requested a review from michaelnchin April 2, 2024 17:37
@kmcginnes
Copy link
Collaborator Author

I'm looking in to the type check failure

@kmcginnes
Copy link
Collaborator Author

I'm looking in to the type check failure

Ok, I was able to fix this by installing the @jest/globals package that was missing.

I have no idea why it was working locally. I must have had that module cached or something.

https://stackoverflow.com/questions/70262724/ts2307-cannot-find-module-jest-globals-or-its-corresponding-type-declaration

Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 88.32117% with 32 lines in your changes are missing coverage. Please review.

Project coverage is 13.95%. Comparing base (2744e93) to head (5163d79).
Report is 2 commits behind head on main.

Files Patch % Lines
...aph-explorer/src/core/StateProvider/filterCount.ts 0.00% 17 Missing and 1 partial ⚠️
...-explorer/src/components/Sidebar/SidebarButton.tsx 0.00% 5 Missing ⚠️
...rer/src/workspaces/GraphExplorer/GraphExplorer.tsx 0.00% 5 Missing ⚠️
packages/graph-explorer/src/hooks/useEntities.ts 0.00% 2 Missing ⚠️
...er/src/modules/EntitiesFilter/useFiltersConfig.tsx 95.91% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #289      +/-   ##
==========================================
+ Coverage   12.15%   13.95%   +1.79%     
==========================================
  Files         411      414       +3     
  Lines       30575    30762     +187     
  Branches      776      878     +102     
==========================================
+ Hits         3717     4293     +576     
+ Misses      26525    26139     -386     
+ Partials      333      330       -3     
Flag Coverage Δ
unittests 13.95% <88.32%> (+1.79%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@vkagamlyk vkagamlyk left a comment

Choose a reason for hiding this comment

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

LGTM

@vkagamlyk vkagamlyk merged commit 361c4ba into aws:main Apr 11, 2024
3 checks passed
@kmcginnes kmcginnes deleted the fix-edge-filtering branch April 12, 2024 15:22
@kmcginnes kmcginnes modified the milestone: Release 1.6.1 May 1, 2024
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.

Entity edge filtering doesn't work
2 participants