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

added numerics and conditions to call numerics component to search #1013

Merged
merged 34 commits into from Jan 10, 2022

Conversation

smitfire
Copy link
Contributor

@smitfire smitfire commented Jan 3, 2022

Fixes #

Screenshot 2022-01-03 at 13 41 56
Screenshot 2022-01-03 at 13 42 02
Screenshot 2022-01-03 at 13 42 05

Description

Added the numerics filter logic but since all the fields with numerics are actually returning strings it is not convenient to test. I have added some preliminary logic to convert the data to actual integers.

The logic for the min, max for the slider range is there too. As well as the show missing checkbox condition but the logic connecting it to the missing query is not added.

How has this been tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ x] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • [ x] My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added necessary unit and integration tests.
  • [x ] I have added screenshots (if applicable), in the comment section.

@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2022

Codecov Report

❗ No coverage uploaded for pull request base (1.7.0-M3@d619c4a). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             1.7.0-M3    #1013   +/-   ##
===========================================
  Coverage            ?   65.95%           
===========================================
  Files               ?       21           
  Lines               ?      611           
  Branches            ?      126           
===========================================
  Hits                ?      403           
  Misses              ?      208           
  Partials            ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d619c4a...30292fa. Read the comment docs.

@smitfire smitfire added this to Review in BlueBrain/nexus via automation Jan 3, 2022
@nicwells nicwells self-requested a review January 6, 2022 09:22
Copy link
Contributor

@nicwells nicwells left a comment

Choose a reason for hiding this comment

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

  • Show missing values doesn't appear to be working for me, seems to have no effect and doesn't disable other controls (related to next point)
  • Clicking on column dropdown arrow results in filter being applied immediately and missing values being removed from results
  • between filter looks a little strange and seems to be repeated (see screenshot below)

Screenshot 2022-01-06 at 10 25 15

  • Couple of styling comments: when numbers is over a certain size it doesn't fit in text box, right side of slider is a bit close to max text box, would be nice to format number of missing values to include comma separator

Screenshot 2022-01-06 at 10 29 00

  • Crashes for me when opening the filter for Bouton Density when pointing to dev - not sure if this is a data thing or not

Dhanesh Neela Mana and others added 7 commits January 6, 2022 21:07
…to 2938-nums

* '2938-nums' of https://github.com/BlueBrain/nexus-web:
  2986 fix styles (#1015)
  Add redirect params to login call back (#1014)
  2644 allow user to select search config (#1011)
  Rewrite useLocalStorage hook and make use of (#1012)
* refactor login, date, num , filter
Copy link
Contributor

@nicwells nicwells 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 :) Just the console.log to remove

src/subapps/search/utils/index.ts Outdated Show resolved Hide resolved
@nicwells nicwells self-requested a review January 10, 2022 13:20
@dhaneshnm dhaneshnm merged commit e5225d2 into 1.7.0-M3 Jan 10, 2022
BlueBrain/nexus automation moved this from Review to Done Jan 10, 2022
@dhaneshnm dhaneshnm deleted the 2938-nums branch January 10, 2022 13:27
dhaneshnm added a commit that referenced this pull request Mar 11, 2022
…1013)

* added numeric and conditions to call numeric component to search



Co-authored-by: Dhanesh Neela Mana <dhanesh.neelamana@epfl.ch>
Co-authored-by: Dhanesh Neela Mana <dhanesh.n.m19@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
4 participants