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

Filter system #172

Merged
merged 10 commits into from
Aug 24, 2021
Merged

Filter system #172

merged 10 commits into from
Aug 24, 2021

Conversation

haihan-lin
Copy link
Collaborator

@haihan-lin haihan-lin commented Aug 17, 2021

Closes #77

@haihan-lin haihan-lin linked an issue Aug 17, 2021 that may be closed by this pull request
@haihan-lin haihan-lin marked this pull request as ready for review August 23, 2021 17:03
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.

Looks good. I'm going to do some testing on the server this afternoon and then I'll approve. I left a couple of suggestions for you, but they shouldn't hold back the PR. Feel free to make the changes if they make sense to you

frontend/src/Components/Charts/HeatMap/SingleHeatRow.tsx Outdated Show resolved Hide resolved
frontend/src/Components/LayoutGenerator.tsx Outdated Show resolved Hide resolved
frontend/src/App.tsx Outdated Show resolved Hide resolved
@haihan-lin
Copy link
Collaborator Author

haihan-lin commented Aug 23, 2021

  • Add a filter board for the app
  • the filter affect the available cases -> question: should the filter affect the "aggregated cases", for example -/4700 to -/90 because the filter changed. Currently this happens but I feel like it is confusing since surgery selection doesn't affect the number.
  • Reset for each filter section
  • Reset button should be disabled when it is at default filter state.
  • cleaned up lineup and semantic ui.

@JackWilb
Copy link
Member

JackWilb commented Aug 23, 2021

Things for me to double check:

  • lineup remnants are fully gone
  • test that it compiles and works as intended
    I noticed issues loading an old state. Saving and loading new states seems fine
    Aggregated cases showed 4712/7. I'm not sure what that means
  • are there any related issues that we can close with this PR

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.

Looks good to me! I noted a couple issues in my comment above that might warrant some further investigation

@haihan-lin
Copy link
Collaborator Author

I noticed issues loading an old state. Saving and loading new states seems fine
Aggregated cases showed 4712/7. I'm not sure what that means

Pretty sure this is because the main interface doesn't "reload" until you click back to render it. But I'll take a closer look on next PR.

@haihan-lin haihan-lin merged commit d11d154 into master Aug 24, 2021
@haihan-lin haihan-lin deleted the filter-system branch August 24, 2021 13:33
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.

Add a way to filter by any variable
2 participants