Gene Panel Modal in Patient View #2782

ngocnn1104 · 2019-10-14T15:11:25Z

Gene Panel Modal in Patient View

Describe changes proposed in this pull request:

Created PatientViewGenePanelModal that makes use of GenePanelModal component;
Created GenesList component that displays a list of genes belonging to a gene panel with filtering, copy and download features - this component is passed as a child component to GenePanelModal at the moment and could be used in a separate page in the future;
Triggered the Gene Panel modal from several places in Patient View: Patient header, Genomic tracks, Mutations table and Copy Number Alterations table;
Implemented unit tests for the new component;
Implemented local end-to-end tests.

GIF

alisman · 2019-10-24T15:09:48Z

src/pages/patientView/PatientViewGenePanelModal/GenesList.tsx

+> {
+    constructor(props: IGenesListProps) {
+        super(props);
+        this.state = {


i think we should stick to just using mobx observables in this project (instead of traditional React component state).
Using both will cause confusion. So, no setState please.

alisman · 2019-10-24T15:10:56Z

src/pages/patientView/PatientViewGenePanelModal/GenesList.tsx

+        this.setState({ filter: value });
+    };
+
+    filterGenes = () => {


this should be computed property. That will avoid unecessary recomputation

alisman · 2019-10-24T15:54:14Z

src/pages/patientView/clinicalInformation/PatientViewPageStore.ts

@@ -947,6 +954,14 @@ export class PatientViewPageStore {
    @action clearErrors() {
        this.ajaxErrors = [];
    }
+
+    @autobind
+    @action toggleModal(name?:string|undefined) {


see comment above. should be on view and needs more specific name

alisman · 2019-10-24T15:55:37Z

src/pages/patientView/genomicOverview/Tracks.tsx


-export default class Tracks extends React.Component<TracksPropTypes, {}> {
+export const DEFAULT_GENOME_BUILD="GRCh37";


not sure but i think this constant already exists elsequere. we should have it one place unless it's only the default for this specific feature

I searched through the source code and only found this constant used in this feature, so I think it's alright to keep it here.

alisman · 2019-10-24T15:56:03Z

src/pages/patientView/genomicOverview/Tracks.tsx

+export default class Tracks extends React.Component<TracksPropTypes, TracksStateTypes> {
+    constructor(props:TracksPropTypes) {
+        super(props);
+        this.state = { genePanelInTooltip: '' };


alisman · 2019-10-24T16:03:08Z

src/shared/components/mutationTable/column/PanelColumnFormatter.tsx

@@ -57,6 +56,27 @@ const getGenePanelIds = (props:PanelColumnFormatterProps) => {
  return [];
 };

+const getGenePanelLinks = (props:PanelColumnFormatterProps) => {


You could make this a React Functional Component and type it as such. Then use it in code via JSX: <GenePanelLinks ... >

src/pages/patientView/PatientViewGenePanelModal/PatientViewGenePanelModal.tsx

src/pages/patientView/SampleManager.tsx

adamabeshouse · 2019-10-24T20:38:52Z

src/pages/patientView/clinicalInformation/ClinicalInformationPatientTable.tsx

@@ -36,6 +37,36 @@ export default class ClinicalInformationPatientTable extends React.Component<ICl
        }
        return ret;
    }
+
+    handleClick = (name:string) => {


just to clarify, I think @alisman means handleClick(name:string) { ... }

src/pages/patientView/clinicalInformation/ClinicalInformationPatientTable.tsx

src/pages/patientView/clinicalInformation/PatientViewPageStore.ts

src/pages/patientView/genomicOverview/tracksHelper.ts

src/shared/components/GenePanelModal/GenePanelModal.tsx

alisman · 2019-10-24T20:50:15Z

@ngocnn1104 we put a lot of comments on this! let me know if you need any clarification or want to discuss.

adamabeshouse

thanks for addressing comments! almost there

src/pages/patientView/PatientViewGenePanelModal/PatientViewGenePanelModal.tsx

adamabeshouse · 2019-10-29T18:14:10Z

src/pages/patientView/PatientViewPage.tsx

+    @action toggleGenePanelModal(genePanelId?:string|undefined) {
+        this.genePanelModal = {
+            isOpen: !this.genePanelModal.isOpen,
+            genePanelId: typeof genePanelId === 'string' ? genePanelId : ''


genePanelId: genePanelId || '' is easier to read

I agree, and have updated according to your suggestion. However, this function was passed directly to handle the onHide event of Bootstrap Modal and onClick event of Bootstrap Button, and whenever the function was called, an event object would get passed to the parent component and be seen as genePanelId. In order to avoid this, I modified the shared GenePanelModal component like below:

@autobind handleClose() { this.props.onHide(); } render() { return ( <Modal show={this.props.show} onHide={this.handleClose} // instead of onHide={this.props.onHide} ...

ah I understand. can you please add a comment explaining that?

src/pages/patientView/clinicalInformation/ClinicalInformationPatientTable.tsx

src/pages/patientView/genomicOverview/Tracks.tsx

adamabeshouse

please add a comment as discussed here #2782 (comment), and other than that looks good thanks!

ngocnn1104 · 2019-10-31T08:59:22Z

please add a comment as discussed here #2782 (comment), and other than that looks good thanks!

done. Thank you!

- Created gene panel modal component in patient view with unit tests - Trigger gene panel modal from genomic tracks and patient header - Trigger gene panel modal from mutations and copy number tables - Implemented local e2e tests & format new files with prettier Signed-off-by: Ngoc Nguyen <nguyennhungoc1104@gmail.com>

pvannierop force-pushed the patientview_gene_coverage_filter branch from b8c5b84 to 9b10cea Compare October 14, 2019 15:22

ngocnn1104 force-pushed the patientview_gene_panel_modal branch 4 times, most recently from 0e4e4ba to 1fab19a Compare October 15, 2019 09:56

ngocnn1104 changed the base branch from patientview_gene_coverage_filter to patientview_filter_genome_overview October 15, 2019 09:57

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 1fab19a to 2ac0966 Compare October 16, 2019 13:51

ngocnn1104 changed the base branch from patientview_filter_genome_overview to gene_panel_download October 16, 2019 13:52

ngocnn1104 added the do not review label Oct 16, 2019

alisman added the DO NOT MERGE label Oct 16, 2019

ngocnn1104 force-pushed the patientview_gene_panel_modal branch 5 times, most recently from 97015d0 to 1ee9305 Compare October 22, 2019 14:53

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 23, 2019 18:58 Inactive

pvannierop force-pushed the gene_panel_download branch from 4843f14 to 5c9e64c Compare October 24, 2019 07:15

pvannierop force-pushed the patientview_gene_panel_modal branch from 1ee9305 to 6f8a259 Compare October 24, 2019 07:15

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 24, 2019 07:15 Inactive

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 24, 2019 13:05 Inactive

ngocnn1104 removed the do not review label Oct 24, 2019

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 24, 2019 13:49 Inactive

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from d35881d to 7d2b3d4 Compare October 24, 2019 13:51

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 24, 2019 13:52 Inactive

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 7d2b3d4 to 50454c4 Compare October 24, 2019 14:07

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 24, 2019 14:08 Inactive

ngocnn1104 requested review from adamabeshouse and alisman October 24, 2019 14:29

alisman reviewed Oct 24, 2019

View reviewed changes

adamabeshouse requested changes Oct 24, 2019

View reviewed changes

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 28, 2019 16:29 Inactive

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 1a29a48 to ff22695 Compare October 29, 2019 09:30

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 29, 2019 09:30 Inactive

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from ff22695 to cd8206c Compare October 29, 2019 10:02

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 29, 2019 10:02 Inactive

adamabeshouse requested changes Oct 29, 2019

View reviewed changes

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from cd8206c to 3ff699f Compare October 30, 2019 10:06

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 30, 2019 10:06 Inactive

adamabeshouse approved these changes Oct 30, 2019

View reviewed changes

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 3ff699f to 53f26fb Compare October 31, 2019 08:55

jjgao temporarily deployed to cbioportal-f-patientvie-gxnf52 October 31, 2019 08:55 Inactive

pvannierop self-assigned this Nov 1, 2019

pvannierop added the RFC45 PatientView: gene panel integration label Nov 1, 2019

ngocnn1104 changed the base branch from gene_panel_download to master December 2, 2019 10:37

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 53f26fb to bed8995 Compare December 2, 2019 10:48

ngocnn1104 removed the DO NOT MERGE label Dec 2, 2019

alisman approved these changes Dec 2, 2019

View reviewed changes

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from bed8995 to 69afd24 Compare December 2, 2019 15:42

ngocnn1104 force-pushed the patientview_gene_panel_modal branch from 69afd24 to ee56e42 Compare December 3, 2019 10:23

ngocnn1104 added the feature label Dec 3, 2019

alisman merged commit cd1ae43 into cBioPortal:master Dec 3, 2019

ngocnn1104 deleted the patientview_gene_panel_modal branch January 25, 2020 16:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gene Panel Modal in Patient View #2782

Gene Panel Modal in Patient View #2782

ngocnn1104 commented Oct 14, 2019 •

edited

alisman Oct 24, 2019

alisman Oct 24, 2019

alisman Oct 24, 2019

alisman Oct 24, 2019

ngocnn1104 Oct 28, 2019

alisman Oct 24, 2019

alisman Oct 24, 2019

adamabeshouse Oct 24, 2019

alisman commented Oct 24, 2019

adamabeshouse left a comment

adamabeshouse Oct 29, 2019

ngocnn1104 Oct 30, 2019

adamabeshouse Oct 30, 2019

adamabeshouse left a comment

ngocnn1104 commented Oct 31, 2019


		export default class Tracks extends React.Component<TracksPropTypes, {}> {
		export const DEFAULT_GENOME_BUILD="GRCh37";

Gene Panel Modal in Patient View #2782

Gene Panel Modal in Patient View #2782

Conversation

ngocnn1104 commented Oct 14, 2019 • edited

Gene Panel Modal in Patient View

GIF

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alisman commented Oct 24, 2019

adamabeshouse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamabeshouse left a comment

Choose a reason for hiding this comment

ngocnn1104 commented Oct 31, 2019

ngocnn1104 commented Oct 14, 2019 •

edited