-
Notifications
You must be signed in to change notification settings - Fork 42
Update/search results layouts details component #3495
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
Update/search results layouts details component #3495
Conversation
…new search details section on click
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3495 +/- ##
==========================================
+ Coverage 45.30% 48.28% +2.98%
==========================================
Files 585 588 +3
Lines 41658 41972 +314
Branches 1324 1377 +53
==========================================
+ Hits 18872 20268 +1396
+ Misses 22595 21534 -1061
+ Partials 191 170 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…s, refactoring modalityhex icon into resuable component
…w component specific file
nellh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, looks like I wasn't checking for white space. how about this - I updated to check for white space dataset names
|
| const subjectCountRangeIsNull = | ||
| JSON.stringify(subjectCountRange) === JSON.stringify([null, null]) | ||
|
|
||
| const labelText = modality_selected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a modalityShortMapping function that does this in a general way.
openneuro/packages/openneuro-app/src/scripts/components/formatting/modality-label.tsx
Lines 1 to 22 in 3fa161f
| /** | |
| * Provide a mapping from the schema names to an expected short name used for OpenNeuro display | |
| */ | |
| export function modalityShortMapping(modality) { | |
| switch (modality) { | |
| case "ieeg": | |
| case "iEEG": | |
| return "iEEG" | |
| case "beh": | |
| return "Behavioral" | |
| case "motion": | |
| return "Motion" | |
| case "micro": | |
| return "Microscopy" | |
| case undefined: | |
| return undefined | |
| case null: | |
| return null | |
| default: | |
| return modality.toUpperCase() | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated this lableText and two other places I was doing the same thing
| const _list = ( | ||
| type: JSX.Element, | ||
| items: (string | JSX.Element)[], | ||
| ): JSX.Element | null => { | ||
| if (items && items.length > 0) { | ||
| return ( | ||
| <div className="result-summary-meta"> | ||
| <label>{type}:</label> | ||
| <div> | ||
| {items.map((item, index) => ( | ||
| <span className="list-item" key={index}> | ||
| {item} | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ) | ||
| } else { | ||
| return null | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sort of thing would probably make sense as a component rather than just a function in the render step. Functions that don't create a closure over any value should generally be defined outside of rendering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new component added for list items in the SearchDetails
…yShortMapping function for formatting modalities in various places in search and dataset page header


Based on newest designs for Annotation feature. This will move the search results tools towards that approach before we make any updates RE: annotation
updates to search layouts