[DT-783] Add study details page #2741
Conversation
s-rubenstein
requested review from
snf2ye and
rjohanek
and removed request for
a team
| <AuthenticatedRoute path='/admin_manage_dar_collections/' component={AdminManageDarCollections} props={props} rolesAllowed={[USER_ROLES.admin]} /> | ||
| <AuthenticatedRoute path='/datalibrary/:query' component={DatasetSearch} props={props} rolesAllowed={[USER_ROLES.admin, USER_ROLES.all]} /> | ||
| <AuthenticatedRoute path='/datalibrary' component={DatasetSearch} props={props} rolesAllowed={[USER_ROLES.admin, USER_ROLES.all]} /> | ||
| <AuthenticatedRoute path='/studies/:studyId' component={StudyDetails} props={props} rolesAllowed={[USER_ROLES.admin, USER_ROLES.all]} /> |
There was a problem hiding this comment.
This is the only new line here, other than that its all linting
| @@ -160,26 +160,26 @@ const NavigationTabsComponent = (props) => { | |||
| return ( | |||
| <div className={`navbar-logged ${orientation === 'vertical' ? 'navbar-vertical' : ''}`}> | |||
| {makeNotifications()} | |||
| <ul className="navbar-main"> | |||
There was a problem hiding this comment.
Entirely linting changes
|
|
||
| interface DatasetSearchFooterProps { | ||
| selectedDatasets: number[]; | ||
| datasets: Dataset[]; | ||
| datasets: DatasetTerm[]; |
There was a problem hiding this comment.
For the elasticsearch terms, this is the right typing (I believe)
| @@ -31,6 +31,21 @@ const styles = { | |||
| }, | |||
| }; | |||
|
|
|||
| export const applyForAccess = async (selected, history) => { | |||
There was a problem hiding this comment.
This got pulled out to be reused elsewhere. I could see this going to a libs or utils file as well - what do you all think?
There was a problem hiding this comment.
In another defunct DAA-related PR, I had pulled this out to a separate component, so I think it makes sense.
There was a problem hiding this comment.
This looks great, thank you! No code suggestions. The linting changes look good, nice catch. There are a couple other study properties we could be showing but would likely require a decision from product on whether we should be showing them here. We could look into adding the following fields. Some are currently indexed, but some may not be and we should look at adding those values to the index:
- study type
- data types
- data submitter and/or email
- phs id, and any other NIH related info we capture
|
@rjohanek it does not maintain search or filters - this was on request from @lstrano-broad when we met with him - he prefers the extra page. |
|
I can add tests |
I think maintaining filter state would be a great feature, but we need to design that and it would be a completely new feature. |
|
I never requested the filters to reset. I agree the filter state needs to maintained when user click back. |
|
@lstrano-broad we'll need to do that as a separate ticket - right now we don't have a mechanism to store filter state between pages, so when you go back to the original URL it has the same filters |
|
(Separate page was Lou's request, to be clear - not the loss of filters. The loss of filters is a side effect that we can fix separately) |
Addresses
Screen.Recording.2024-12-03.at.3.33.58.PM.mov
Summary
Have you read Terra's Contributing Guide lately? If not, do that first.