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

Add Clinical Attributes to Mutations Tab in Results View #3793

Merged

Conversation

Rajat-Sirohi
Copy link
Contributor

@Rajat-Sirohi Rajat-Sirohi commented Jun 9, 2021

Fix cBioPortal/cbioportal#8657 + first two of cBioPortal/cbioportal#8716

Changes:

  • Add clinical attributes to columns selection on Mutations Tab in Results View (loaded lazily)
  • Replace react-bootstrap drop-down menu with custom one from pages/studyView/.../AddChartByType
    (only replaces drop-down menu for Mutations Tab in Results View, not all Mutation Tables)
  • Minor fixes/refactoring on 'Deselect all' button in FixedHeaderTable.tsx

Currently, neither the group comparison nor custom charts clinical attributes are included, since they need individualized methods of fetching the clinical data and testing them proves difficult without a login. This should be addressed in a later PR.

Old drop-down menu:

old_dropdown

New drop-down menu:

new_mutations_tab
new_clinical_tab

@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 5 times, most recently from 17206ca to bd7bb36 Compare June 10, 2021 17:48
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 2 times, most recently from 725c10d to 2c4baff Compare June 11, 2021 07:37
Copy link
Member

@onursumer onursumer left a comment

Choose a reason for hiding this comment

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

Thanks for the refactoring! Added a minor comment about the custom dropdown component.

@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 2 times, most recently from 4b0b8a8 to cd3395f Compare June 15, 2021 17:34
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 3 times, most recently from 9c310d6 to 6cd6c81 Compare June 17, 2021 17:59
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/shared/cache/ClinicalAttributeCache.ts Show resolved Hide resolved
src/shared/components/mutationTable/MutationTable.tsx Outdated Show resolved Hide resolved
@inodb
Copy link
Member

inodb commented Jun 21, 2021

@Rajat-Sirohi Thanks for implementing this! Is it correct that the filtering with the search field doesn't currently work?

E.g.:

Screen Shot 2021-06-21 at 10 27 33 AM

@Rajat-Sirohi
Copy link
Contributor Author

@inodb Currently, since clinical attribute datatypes are not always strings, I have not implemented any filter for them. If you like, I can attach a filter for those columns with string datatypes?

@inodb
Copy link
Member

inodb commented Jun 21, 2021

@Rajat-Sirohi if it's easy to do it for all string types it would be a nice enhancement

@inodb
Copy link
Member

inodb commented Jun 21, 2021

@Rajat-Sirohi i know you'll be adding filtering per column as well in the future, which would address it as well. So only if it's easy to add (CC @Luke-Sikina thoughts?)

@Luke-Sikina
Copy link
Member

@Rajat-Sirohi i know you'll be adding filtering per column as well in the future, which would address it as well. So only if it's easy to add (CC @Luke-Sikina thoughts?)

I think Nikki mentioned that he wanted to keep the global filtering, so we should definitely make it work for the string typed columns

@onursumer
Copy link
Member

@inodb Currently, since clinical attribute datatypes are not always strings, I have not implemented any filter for them. If you like, I can attach a filter for those columns with string datatypes?

We need to make sure that search is functional for not-yet-loaded clinical data. I can't remember the current behavior for lazy loaded columns, but it may not work as desired by default.

@Rajat-Sirohi
Copy link
Contributor Author

@onursumer Thanks for the heads-up! Luckily, it looks like filtering via search automatically traverses all the pages in the table, which activates the respective fetch calls for the currently visible lazy loaded columns. So no need to change anything.

@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch from 6cd6c81 to dbf85d8 Compare June 21, 2021 18:25
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 3 times, most recently from 994073b to 014f9c5 Compare June 24, 2021 17:41
Copy link
Member

@onursumer onursumer 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, thanks! Just added a few more minor comments.

src/pages/resultsView/mutation/AddColumns.tsx Show resolved Hide resolved
src/pages/resultsView/mutation/AddColumns.tsx Outdated Show resolved Hide resolved
src/shared/components/mutationTable/MutationTable.tsx Outdated Show resolved Hide resolved
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch 4 times, most recently from bb6e2d8 to d0107da Compare June 30, 2021 18:07
@inodb inodb added the feature label Jun 30, 2021
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch from d0107da to 61e2ba0 Compare June 30, 2021 19:13
@Rajat-Sirohi Rajat-Sirohi marked this pull request as draft June 30, 2021 19:15
@Rajat-Sirohi Rajat-Sirohi force-pushed the add-clinical-attributes-mutations-tab branch from 61e2ba0 to c0be78f Compare June 30, 2021 19:49
@Rajat-Sirohi Rajat-Sirohi marked this pull request as ready for review June 30, 2021 20:29
@inodb
Copy link
Member

inodb commented Jun 30, 2021

Failing tests seem unrelated: https://app.circleci.com/pipelines/github/cBioPortal/cbioportal-frontend/5099/workflows/e5e694fb-7b7b-4b0b-bacd-6f2b28eb964a/jobs/103269/artifacts. Somehow the heatmaps tab started disappearing, so that's the reason why quite a few screenshots are failing

@inodb inodb merged commit ee9c19c into cBioPortal:master Jun 30, 2021
@inodb
Copy link
Member

inodb commented Jun 30, 2021

Thanks so much @Rajat-Sirohi ! Really nice work 🥇 !

@Rajat-Sirohi Rajat-Sirohi deleted the add-clinical-attributes-mutations-tab branch July 1, 2021 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants